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

feat(deps)!: upgrade odm layer to MongDB Rust driver v3.0.0 #30

Merged
merged 7 commits into from
Jul 9, 2024

Conversation

douggynix
Copy link
Contributor

@douggynix douggynix commented Jul 6, 2024

This pull request is related to that issue below I have opened to switch to the newer version of Mongodb Rust Driver v3.0.0
Issue : #29

Mongodb team introduced a lot of broken changes as a regard to the former api in 2.x. Some methods have their arguments reduced, or some errors have been renamed. A lot of refactoring resulted in having build errors for features that have been also removed in Cargo.

This pull request is opened to address those.
I have run the Unit test and it ran successfully. Feel free to review, fix, accept or deny this PR.

Here is one of the PR from Mongodb Driver Rust team from which originates those broken changes from the APIs: mongodb/mongo-rust-driver#1046
FindOptions, FindOneOptions and so on are removed from find method arguments and encapsulated by chain calling explicitly a with_options()

db.collection.find_one(doc).with_options(find_one_options) ;
db.collection.insert_one(&doc).with_options(insert_options).session(session);

image

Additional PR from mongodb rust team with broken changes from the former 2.x api: //See driver pull request change for errors: mongodb/mongo-rust-driver#1102

@douggynix douggynix changed the title upgrade odm layer to mongodb rust driver v3.0.0 upgrade odm layer to Mongodb Rust driver v3.0.0 Jul 6, 2024
@douggynix douggynix changed the title upgrade odm layer to Mongodb Rust driver v3.0.0 Upgrade odm layer to Mongodb Rust driver v3.0.0 Jul 6, 2024
Cargo.toml Outdated Show resolved Hide resolved
douggynix and others added 3 commits July 6, 2024 10:23
Since we are still pre 1.0.0, it's fine to bump the minor number only. Cargo is treating those as breaking.

Co-authored-by: Benoît Cortier <[email protected]>
@douggynix
Copy link
Contributor Author

@CBenoit : I am done with my changes. Feel free to do the last reviews for refactoring if needed in order to have it merged.
I have also tested my changes as an external depency against a code based i am working on which depends on this crate with the mongodb rust driver v3.0.0. and it works

@douggynix
Copy link
Contributor Author

Any update or another review to have this merged?

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Thank you! I went through it. I’m a bit surprised by the formatting, could you try to run cargo fmt again?

Cargo.toml Outdated Show resolved Hide resolved
Comment on lines -372 to +377
let index_doc = if index.keys.iter().any(|ind| matches!(ind, IndexKey::TextIndex(_))) {
let index_doc = if index
.keys
.iter()
.any(|ind| matches!(ind, IndexKey::TextIndex(_)))
{
Copy link
Member

Choose a reason for hiding this comment

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

question: Do you know why the formatting was changed here? Is it the result of cargo fmt using the stable channel? I see other similar places.

Copy link
Contributor Author

@douggynix douggynix Jul 9, 2024

Choose a reason for hiding this comment

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

The formatting has been changed due to cargo.
I just ran cargo fmt as simple as that and it reformats it that way automatically.

cargo fmt --verbose 

I am using rust tool chains version 1.79.0

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I just realized that our CI was dead and we were not checking the formatting anymore.

Copy link
Contributor Author

@douggynix douggynix Jul 9, 2024

Choose a reason for hiding this comment

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

I noticed the CI was dead as well. We can address it from another PR or this one?

Copy link
Member

Choose a reason for hiding this comment

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

I opened a PR for that already: #31

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

This is looking good to me. I’ll merge and release a new version.

@CBenoit CBenoit changed the title Upgrade odm layer to Mongodb Rust driver v3.0.0 feat(deps)!: upgrade odm layer to MongDB Rust driver v3.0.0 Jul 9, 2024
@CBenoit CBenoit merged commit 66a3a00 into Devolutions:master Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants