Skip to content
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

Add "sensible" default behaviour for the CLI update command #17

Merged
merged 5 commits into from
Mar 27, 2024

Conversation

dmoverton
Copy link
Contributor

@dmoverton dmoverton commented Mar 27, 2024

Describe your changes

Attempt to provide some "sensible" default behaviour for the update command if no command line parameters are given.
The idea is to have something that will work well for most users when run from ddn quickstart or ddn dev.

  • Default behaviour is now to use the validator schema if a collection has one.
  • If there is no validator schema then sample documents from the collection instead.
  • If no --sample-size is given on the command line then default to a sample size of 10.
  • Add new command line flag --no-validator-schema which overrides the default behaviour and prevents the command from attempting to use validator schemas.

Issue ticket number and link

MDB-83

@dmoverton dmoverton self-assigned this Mar 27, 2024
Some(schema_bson) => {
let validator_schema =
from_bson::<ValidatorSchema>(schema_bson.clone()).map_err(|err| {
MongoAgentError::BadCollectionSchema(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the collection has a something in $jsonSchema that can't be parsed as a ValidatorSchema then we treat this as an error. Perhaps we should just log this and continue, in which case we would fall back to sampling for this collection. But I think it's probably better to treat it as an error.

Copy link
Contributor

@daniel-chambers daniel-chambers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

CHANGELOG.md Outdated
Comment on lines 5 to 12
- Use separate schema files for each collection
- Don't sample from collections that already have a schema
- Changes to `update` CLI command:
- new default behaviour:
- attempt to use validator schema if available
- if no validator schema then sample documents from the collection
- don't sample from collections that already have a schema
- if no --sample-size given on command line, default sample size is 10
- new option --no-validator-schema to disable attempting to use validator schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we link to the PRs for these changelog entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@dmoverton dmoverton merged commit cbc8a86 into main Mar 27, 2024
1 check passed
@dmoverton dmoverton deleted the dmoverton/sensible-defaults branch March 27, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants