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

GODRIVER-1937 Update legacy ListCollections to support the BatchSize option for server version 2.6 #656

Merged

Conversation

matthewdale
Copy link
Collaborator

Support the BatchSize option for the ListCollections call for server version 2.6

Changes:

  • Use the cursor.batchSize field to set the NumberToReturn in the legacy list collections command for server version 2.6
  • Re-enable getMore/killCursors command tests for server version 2.6

@matthewdale matthewdale force-pushed the godriver1937-fix-legacy-batchsize branch from 72b1ea7 to dcf9c12 Compare April 29, 2021 18:57
Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

The "batch size" test above "getMore commands are monitored" could also probably be looped into this change.

mongo/integration/database_test.go Outdated Show resolved Hide resolved
mongo/integration/database_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@divjotarora divjotarora left a comment

Choose a reason for hiding this comment

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

Thanks for investigating this and putting up a fix quickly! I echo all of @iwysiu's comments so LGTM assuming you address those.

mongo/integration/database_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

LGTM pending other comments!

mongo/integration/database_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

@iwysiu I can't seem to respond to your other comment about min server version in this review now that I've updated the code, but I've removed the unnecessary 2.6 min server version check.

mongo/integration/database_test.go Outdated Show resolved Hide resolved
@matthewdale matthewdale requested a review from iwysiu May 4, 2021 18:19
Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

@matthewdale Mostly looks good with a couple nits!
Not sure if you saw the comment on my last review about trying to run the "batch size" test on line 224 on server version 2.6 too. Since that test specifically checks that "batchSize" gets sent, it makes sense to me to try to include it

mongo/integration/database_test.go Outdated Show resolved Hide resolved
mongo/integration/database_test.go Outdated Show resolved Hide resolved
@matthewdale matthewdale requested a review from iwysiu May 4, 2021 20:53
@matthewdale matthewdale merged commit 899e5cd into mongodb:master May 4, 2021
matthewdale added a commit that referenced this pull request May 5, 2021
stlimtat pushed a commit to stlimtat/mongo-go-driver that referenced this pull request May 17, 2021
* 'master' of https://github.com/mongodb/mongo-go-driver: (39 commits)
  GODRIVER-2004 Add Versioned API connection examples for Docs (mongodb#665)
  GODRIVER-1961 Run OCSP tests against RHEL 7.0 (mongodb#664)
  GODRIVER-1844 finer precision for getSecondsSinceEpoch (mongodb#666)
  GODRIVER-1973 create internal copy of aws v4 signing code (mongodb#657)
  GODRIVER-1951 Update the Go version for Evergreen builds to 1.16 (mongodb#663)
  GODRIVER-1949 add more ignored killAllSessions errors for unified tes… (mongodb#658)
  GODRIVER-1963 remove dropDatabase result (mongodb#660)
  GODRIVER-1180 Remove legacy transform functions from mongo (mongodb#583)
  GODRIVER-1937 Update legacy ListCollections to support the BatchSize option for server version 2.6 (mongodb#656)
  GODRIVER-1933 remove xtrace from shell scripts (mongodb#661)
  fix README error handling of FindOne (mongodb#636)
  GODRIVER-1938 update mongocryptd serverSelectionTimeout to 10 seconds (mongodb#659)
  GODRIVER-1925 Surface cursor errors in DownloadStream fillBuffer (mongodb#653)
  GODRIVER-1955 create labeledError interface (mongodb#651)
  GODRIVER-1947 Unmarshal unicode surrogate pairs correctly in UnmarshalExtJSON. (mongodb#649)
  Changed order of actions in ObjectIDFromHex func (mongodb#637)
  GODRIVER-1750 Ensure contexts are always cancelled during server monitoring (mongodb#654)
  GODRIVER-1931 Sync new cursors and SDAM LB tests (mongodb#655)
  GODRIVER-1981 Sync new transactions tests (mongodb#652)
  GODRIVER-1931 Run tests against LBs in Evergreen (mongodb#648)
  ...
tsedgwick pushed a commit to mongodb-forks/mongo-go-driver that referenced this pull request May 27, 2021
tsedgwick pushed a commit to mongodb-forks/mongo-go-driver that referenced this pull request Jun 1, 2021
@matthewdale matthewdale deleted the godriver1937-fix-legacy-batchsize branch July 13, 2021 05:23
faem pushed a commit to kubedb/mongo-go-driver that referenced this pull request Mar 17, 2022
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.

5 participants