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

DRIVERS-2865: Require drivers to always send out bypassDocumentValidation #1703

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Oct 30, 2024

Please complete the following before merging:

  • Update changelog.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, sharded
    clusters, and serverless).

This effectively reverts #266 and adds tests for the behaviour. I've also decided to remove the rationale for sending bypassDocumentValidation: true in the bulk write specification as this difference no longer exists.

The new tests are grouped in a single file to make it easier for drivers to skip this particular test until they've had a chance to make the changes. No other test (aside client bulkWrite which already implements the new behaviour) use bypassDocumentValidation: false, so this change should not have any side effects in drivers.

@alcaeus alcaeus requested a review from jmikola October 30, 2024 12:54
@alcaeus alcaeus requested a review from a team as a code owner October 30, 2024 12:54
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Test changes LGTM but I can't verify "Test changes in at least one language driver." Add a link to the related PR if possible.

@alcaeus alcaeus force-pushed the drivers-2865-bypassdocumentvalidation-changes branch from 230f1fa to 155bce9 Compare November 4, 2024 08:22
@alcaeus
Copy link
Member Author

alcaeus commented Nov 4, 2024

Test changes LGTM but I can't verify "Test changes in at least one language driver." Add a link to the related PR if possible.

I tested this locally, but since we rely on libmongoc's bulk write implementation for all inserts, I was only testing the negative case. I was able to confirm that aggregate with $out works as expected.

@alcaeus alcaeus merged commit b1b5a7f into mongodb:master Nov 4, 2024
5 checks passed
@alcaeus alcaeus deleted the drivers-2865-bypassdocumentvalidation-changes branch November 4, 2024 08:25
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