feat: allow opting out of schema sampling with config disableSchemaSampling=true MONGOSH-2370 #2503
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new disableSchemaSampling user configuration flag to opt out of sampling collection schemas when using the new autocomplete feature, and wires it through the shell state, REPL initialization, autocomplete entry point, and tests.
- Introduce
disableSchemaSamplinginCliUserConfigand its validator - Update
ShellInstanceState.getAutocompletionContextand the autocomplete initializer to respect the new flag - Pass the flag from the REPL, and add tests to cover enabling/disabling schema sampling
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/index.ts | Add disableSchemaSampling property and validation |
| packages/shell-api/src/shell-instance-state.ts | Wire flag into getAutocompletionContext logic |
| packages/cli-repl/src/mongosh-repl.ts | Forward config to initNewAutocompleter |
| packages/cli-repl/src/mongosh-repl.spec.ts | Test behavior when sampling is disabled/enabled |
| packages/autocomplete/src/index.ts | Accept and pass through the new option |
Comments suppressed due to low confidence (2)
packages/types/src/index.ts:523
- [nitpick] Add a JSDoc comment describing what
disableSchemaSamplingdoes (e.g. "If true, skips querying server for schema samples in autocomplete").
disableSchemaSampling = false;
packages/shell-api/src/shell-instance-state.ts:418
- [nitpick] Consider updating the method’s docstring or interface comment to explain the new
disableSchemaSamplingparameter and its default behavior.
public getAutocompletionContext({
lerouxb
changed the title
|
|
||
| // TODO: we need MQL support in mongodb-ts-autocomplete in order for it | ||
| // to autocomplete collection field names | ||
| wantQueryOperators = false; |
There was a problem hiding this comment.
drive-by but I realised this test should actually be passing by now (with the env var set, obviously
| @@ -2509,9 +2510,25 @@ describe('CliRepl', function () { | |||
| if (!(testServer as any)?._opts.args?.includes('--auth')) { | |||
| // make sure there are some collections we can autocomplete on below | |||
| input.write('db.coll.insertOne({})\n'); | |||
| input.write('db.movies.insertOne({})\n'); | |||
| input.write('db.movies.insertOne({ year: 1 })\n'); | |||
There was a problem hiding this comment.
Something I haven't really given much thought before, but the mql library won't autocomplete under fields that it doesn't know about:
const query: Query<{/* empty schema */}> = {
year: {
$g // you can't auto-complete here
}
}
| @@ -71,10 +71,7 @@ async function waitEval(bus: MongoshBus) { | |||
| } | |||
|
|
|||
| async function waitCompletion(bus: MongoshBus) { | |||
| await Promise.race([ | |||
There was a problem hiding this comment.
This timeout was just always going to be flaky and pressing tab twice doesn't necessarily mean it completed a potentially slow operation anyway. tabtab() always relied on 2 * 250ms being enough.
| @@ -510,6 +505,8 @@ class MongoshNodeRepl implements EvaluationListener { | |||
| results = results.filter( | |||
| (result) => !CONTROL_CHAR_REGEXP.test(result) | |||
| ); | |||
| // emit here so that on nextTick the results should be output | |||
There was a problem hiding this comment.
OMG. autocomplete became async and there's another await.
| @@ -293,7 +293,9 @@ export async function completer( | |||
| // from https://github.com/mongodb-js/devtools-shared/commit/e4a5b00a83b19a76bdf380799a421511230168db | |||
| function satisfiesVersion(v1: string, v2: string): boolean { | |||
| const isGTECheck = /^\d+?\.\d+?\.\d+?$/.test(v2); | |||
| return semver.satisfies(v1, isGTECheck ? `>=${v2}` : v2); | |||
| return semver.satisfies(v1, isGTECheck ? `>=${v2}` : v2, { | |||
| includePrerelease: true, | |||
There was a problem hiding this comment.
> semver.satisfies('8.2.0-alpha-2686-g3770008', '>=8.0', {
... includePrerelease: true,
... });
true
>
> semver.satisfies('8.2.0-alpha-2686-g3770008', '>=8.0', {
... includePrerelease: false, // the default
... });
false
Didn't realise this was only failing on latest and that latest means latest alpha, not latest production release.
This is only relevant for the new autocompleter which is currently feature-flagged and only accessible if you run mongosh with the environment variable USE_NEW_AUTOCOMPLETE=true
Without it set we'll still read database and collection names like we've always done. With this set we'll just assume an empty collection/schema, so you won't see field names autocomplete.