-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Update MongoDB driver to mongodb-driver-legacy:5.4.0 #34100
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
Conversation
| mockito_core : "org.mockito:mockito-core:3.7.7", | ||
| mockito_inline : "org.mockito:mockito-inline:4.5.1", | ||
| mongo_java_driver : "org.mongodb:mongo-java-driver:3.12.11", | ||
| mongo_java_driver : "org.mongodb:mongodb-driver-legacy:5.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To minimize the changes, I left the name of the library as is, but happy to change it to mongodb-driver-legacy if that makes sense.
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
| private DBCursor createCursor(GridFS gridfs) { | ||
| if (spec.filter() != null) { | ||
| DBObject query = (DBObject) JSON.parse(spec.filter()); | ||
| DBObject query = BasicDBObject.parse(spec.filter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Extended JSON syntax accepted by BasicDBObject#parse is mostly consistent with the syntax accepted by JSON#parse. To understand whether this might cause compatibility issues, can anyone point me to how spec.filter() is initialized? Does the filter come from the application or is it generated by the framework, and if the later, where in the code does that happen?
|
assign set of reviewers |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @robertwb for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Apologies for not being able to get all the checks to pass. Would appreciate some assistance. |
|
The failed test is From failed test, go to "Summary", One can see "For more details on these failures, see this check" with a link, then the stack trace for failed test is there. Also, there is "JUnit Test Results" uploaded in Artifact Section and one can download |
|
Reminder, please take a look at this pr: @robertwb @Abacn @johnjcasey |
|
The 5.4.0 driver was recently released, and I updated the dependency to reflect that. Note that 5.4.0, for the first time, includes a BOM. I'm unsure how to reference the BOM in this project, but perhaps maintainers can advise. |
|
Also unclear why this error is happening: Can anyone advise? |
This is a gradle-dependency-analyze check. It suggests in ":sdks:java:extensions:sql" it is still mongodb-driver-core instead of mongodb-driver-legacy are actually used one |
|
@Abacn I tried various combinations of dependencies, but have not been able to find the right combination that works. I would appreciate if someone with more knowledge of the build system could give it a shot, but I understand if this PR is not top of mind for the maintainers. |
|
Reminder, please take a look at this pr: @robertwb @Abacn @johnjcasey |
|
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment R: @kennknowles for label java. Available commands:
|
|
R: @Abacn (keeping sticky since Yi has started to review) |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions. |
|
@jyemin could you address the error and resolve the conflict so that the review can continue ? Please let us know if you need additional help. |
|
waiting on author |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
Upgrade MongoDB Driver to latest available release.
Note that as of 4.0 the mongo-java-driver uber jar is no longer published, so using mongodb-driver-legacy (which will pull in transitive dependencies) in its place.
I'll also note that I wasn't able to build the project locally without errors so I'm using the PR to check whether everything still works.
#32088
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.