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

Bugfix/arsn 371 useunifiedtopology #2179

Closed

Conversation

williamlardier
Copy link
Contributor

Add an option to support and enable this feature, as it is not clear, depending on the driver version, if this flag defaults to true or not.

@bert-e
Copy link
Contributor

bert-e commented Oct 12, 2023

Hello williamlardier,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Oct 12, 2023

Incorrect fix version

The Fix Version/s in issue ARSN-371 contains:

  • 8.1.113

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

  • 8.1.114

Please check the Fix Version/s of ARSN-371, or the target
branch of this pull request.

@codecov-commenter
Copy link

Codecov Report

Merging #2179 (017b3be) into development/8.1 (17b5bbc) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@               Coverage Diff                @@
##           development/8.1    #2179   +/-   ##
================================================
  Coverage            64.34%   64.34%           
================================================
  Files                  210      210           
  Lines                16391    16394    +3     
  Branches              3320     3321    +1     
================================================
+ Hits                 10547    10549    +2     
- Misses                5829     5830    +1     
  Partials                15       15           
Files Coverage Δ
...orage/metadata/mongoclient/MongoClientInterface.js 63.19% <75.00%> (+<0.01%) ⬆️

@francoisferrand
Copy link
Contributor

not sure what this brings: we want to use this mode, it should not be disabled, and is the default in the version of arsenal where you add it...

@williamlardier
Copy link
Contributor Author

@francoisferrand it is not clearly documented if the option is set to true by default, if the option is still useful, or not. The only hint is here: https://www.mongodb.com/docs/drivers/node/current/whats-new/#mongoclientoptions-interface

In some other places like here https://www.mongodb.com/docs/atlas/resilient-application/, they still use it.
So it won't hurt making sure this option is set to true when we explicitly want to have it.

Copy link
Contributor

@benzekrimaha benzekrimaha left a comment

Choose a reason for hiding this comment

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

LGTM

@francoisferrand
Copy link
Contributor

The link you mention indicates it is not needed anymore (which is a hint for "should not be used" or "is useless"); and looking at the (official) documentation [1] it is not present anymore, since 5.0 at least...

[1] https://mongodb.github.io/node-mongodb-native/5.0/interfaces/MongoClientOptions.html

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