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-2802: Require 4.3.1+ when using failCommand errorLabels option #1489

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Jan 3, 2024

https://jira.mongodb.org/browse/DRIVERS-2802

This also improves version info in the failCommand docs within the Unified Test Format spec.

Please complete the following before merging:

  • Update changelog.
  • Make sure there are generated JSON files from the YAML test files.
  • 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).

POC: mongodb/mongo-c-driver#1508

libmongoc patch build: https://spruce.mongodb.com/version/659d96522fbabe331b61658a

@jmikola jmikola requested review from a team as code owners January 3, 2024 20:16
@jmikola jmikola requested review from alcaeus and isabelatkinson and removed request for a team January 3, 2024 20:16
jmikola added a commit to jmikola/mongo-c-driver that referenced this pull request Jan 5, 2024
Synced with mongodb/specifications#1489

No changes were needed for retryable writes prose tests. libmongoc only implements prose test 3, which already requires MongoDB 6.0+.

Note: with_transaction/callback-retry.json is unrelated but was missed in CDRIVER-2975.
jmikola added a commit to jmikola/mongo-c-driver that referenced this pull request Jan 8, 2024
Synced with mongodb/specifications#1489

No changes were needed for retryable writes prose tests. libmongoc only implements prose test 3, which already requires MongoDB 6.0+.

Note: with_transaction/callback-retry.json is unrelated but was missed in CDRIVER-2975.
jmikola added a commit to jmikola/mongo-c-driver that referenced this pull request Jan 9, 2024
Synced with mongodb/specifications#1489

No changes were needed for retryable writes prose tests. libmongoc only implements prose test 3, which already requires MongoDB 6.0+.

Note: with_transaction/callback-retry.json is unrelated but was missed in CDRIVER-2975. This test currently fails and has been skipped (see: CDRIVER-4811).
jmikola added a commit to jmikola/mongo-c-driver that referenced this pull request Jan 9, 2024
Synced with mongodb/specifications#1489

No changes were needed for retryable writes prose tests. libmongoc only implements prose test 3, which already requires MongoDB 6.0+.

Note: with_transaction/callback-retry.json is unrelated but was missed in CDRIVER-2975. This test currently fails and has been skipped (see: CDRIVER-4811).
Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The references to SERVER tickets and versions in the Unified Test Format is much appreciated.

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

lgtm with one suggestion, no need for another round

specification. This test requires MongoDB 4.2.9+ for ``blockConnection`` support in the failpoint.
specification.

This test requires MongoDB 4.3.4+ for both the ``errorLabels`` and
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 4.3.4 here rather than 4.3.1? I suggest using the latter for consistency if the requirements are the same as the other tests changed in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Since no one should be testing with any 4.3 release, can we just bump to 4.4?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why 4.3.4 here rather than 4.3.1

4.3.4 is the first release where both errorLabels _and blockConnection were available (see: SERVER-41070).

Since no one should be testing with any 4.3 release, can we just bump to 4.4?

IMO, using exact versions when they're accessible is helpful to preserve history. Seeing the versions before I started on this PR is what clued me in that https://github.com/mongodb/mongo/wiki/The-%22failCommand%22-fail-point and the Unified Test Format spec needed to be updated.

I suppose something to consider down the line would be to periodically bump server versions in spec/prose tests after final versions are released (e.g. after 7.0 is cut, we should have no server version requirements for pre-7.0 unstable releases in existing tests) -- but I don't need to be involved in that.

@jmikola
Copy link
Member Author

jmikola commented Jan 10, 2024

The references to SERVER tickets and versions in the Unified Test Format is much appreciated.

FWIW, I also managed to get https://github.com/mongodb/mongo/wiki/The-%22failCommand%22-fail-point updated (thanks @Schubes).

jmikola added a commit to jmikola/mongo-c-driver that referenced this pull request Jan 10, 2024
Synced with mongodb/specifications#1489

Updates retryable writes prose test 3 to relocate the errorLabels field when configuring a fail point.

Note: with_transaction/callback-retry.json is unrelated but was missed in CDRIVER-2975. This test currently fails and has been skipped (see: CDRIVER-4811).
@jmikola jmikola merged commit 5fc23f4 into mongodb:master Jan 10, 2024
4 checks passed
@jmikola jmikola deleted the drivers-2802 branch January 10, 2024 18:40
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.

4 participants