-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update mongosh to 2.0.0 and driver to 6.0.0 VSCODE-453 #592
Conversation
@@ -1089,6 +1090,11 @@ | |||
"xvfb-maybe": "^0.2.1", | |||
"yargs-parser": "^20.2.9" | |||
}, | |||
"overrides": { | |||
"mongodb-connection-model": { | |||
"@mongodb-js/compass-utils": "0.3.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mongodb-js/compass-utils
doesn’t export getStoragePaths()
in the most recent version anymore which makes require()
ing mongodb-connection-model
fail, technically a breaking change and maybe we should have actually labelled mongodb-js/compass@2c91060 as feat!:
rather than chore(fs):
…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that vscode is so coupled with compass. Actually I didnt know that we are using legacy compass connections in vscode at all.
getConnectionTitle, | ||
extractSecrets, | ||
mergeSecrets, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are now exported by @mongodb-js/connection-storage
, and we may be able to use it, but that has a hard, non-optional/non-lazy dependency on keytar
, so I don’t think we can use it here. Together with the item below, that made it seem very reasonable to just use a prior version of mongodb-data-service
and take care of this issue when we actually do the VSCode connection improvements project.
getConnectionTitle, | ||
extractSecrets, | ||
mergeSecrets, | ||
convertConnectionModelToInfo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could copy the source code for this one, but then we’d still have a dependency on mongodb-connection-model
and would need to use the outdated version of that (or, we could, recursively, also add the source code for that entire package).
opts: ConnectionOptionsFromLegacyDS | ||
): ConnectionOptionsFromCurrentDS { | ||
// Ensure that, at most, the types for OIDC mismatch here. | ||
return opts as Omit<typeof opts, 'oidc'>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t have proper OIDC support yet anyway
// https://github.com/mongodb-js/compass/blob/main/packages/compass-field-store/src/stores/store.js#L193 | ||
{ limit: 1 } | ||
) | ||
.toArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseSchema
would want a cursor from a 5.x driver right now, should probably update this as well at some point but since we only use a single document anyway .toArray()
is a nice way to decouple the types here
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes