chore: add orgId and projectid #159
Conversation
src/tools/tool.ts
Outdated
| if ("orgId" in data && typeof data.orgId === "string" && data.orgId.trim() !== "") { | ||
| toolMetadata.orgId = data.orgId; | ||
| } | ||
| } catch (error) { |
There was a problem hiding this comment.
[q] when might catch happen?
There was a problem hiding this comment.
hopefully never I just don't want telemetry to cause any issues but can remove
There was a problem hiding this comment.
I see no method that might have side effects, which line might throw? perhaps argsShape.safeParse?
perhaps the try catch is better suited for emitToolEvent, no?
There was a problem hiding this comment.
it's just that we could still emit if this goes wrong, but no strong opinions
Co-authored-by: Gagik Amaryan <me@gagik.co>
| })); | ||
|
|
||
| beforeEach(() => { | ||
| integration.mcpServer().userConfig.connectionString = mdbIntegration.connectionString(); | ||
| integration.mcpServer().userConfig.telemetry = "disabled"; // Ensure telemetry stays disabled |
There was a problem hiding this comment.
[q] do we need to do this for atlas as well?
There was a problem hiding this comment.
if we are changing this value on the fly, it would be better for the test that changes it to change it back.
There was a problem hiding this comment.
I've been testing this, need to change it slightly, will udpate
There was a problem hiding this comment.
updated, and atlas is not emitting, only mdbtools were missing
tests/integration/helpers.ts
Outdated
|
|
||
| beforeAll(async () => { | ||
| // GET DO_NOT_TRACK value | ||
| oldDoNotTrackValue = process.env.DO_NOT_TRACK; | ||
| process.env.DO_NOT_TRACK = "1"; | ||
| const userConfig = getUserConfig(); |
There was a problem hiding this comment.
Instead of keeping track of DO_NOT_TRACK, why not?
| const userConfig = getUserConfig(); | |
| const userConfig = { | |
| ...getUserConfig(), | |
| telemetry: "disabled" | |
| }; |
There was a problem hiding this comment.
actually, let me try to sort our default test config everywhere
There was a problem hiding this comment.
I think I had this somewhere too, but I can add it if 3eee38c doesn't work
There was a problem hiding this comment.
this won't work too cause telemetry imports config so it's not injected, only env variables would work or we can refactor
Uh oh!
There was an error while loading. Please reload this page.