Improvement/bb 632 bump mongo db driver 6 #2599

benzekrimaha · 2024-12-10T09:53:14Z

bert-e · 2024-12-10T09:53:18Z

Hello benzekrimaha,

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.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2024-12-10T09:53:59Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

codecov · 2024-12-10T10:27:29Z

Codecov Report

Attention: Patch coverage is 80.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 71.11%. Comparing base (070472c) to head (287b942).
Report is 11 commits behind head on development/9.0.

Files with missing lines	Patch %	Lines
lib/util/LocationStatusManager.js	68.75%	5 Missing ⚠️
...s/notification/configManager/MongoConfigManager.js	83.33%	2 Missing ⚠️
extensions/utils/LocationStatusStream.js	81.81%	2 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
extensions/oplogPopulator/OplogPopulator.js	`89.14% <100.00%> (ø)`
extensions/replication/tasks/ReplicateObject.js	`91.49% <100.00%> (ø)`
lib/api/BackbeatAPI.js	`90.10% <100.00%> (+3.05%)`	⬆️
...s/notification/configManager/MongoConfigManager.js	`97.80% <83.33%> (+14.82%)`	⬆️
extensions/utils/LocationStatusStream.js	`92.59% <81.81%> (+19.86%)`	⬆️
lib/util/LocationStatusManager.js	`77.51% <68.75%> (+17.12%)`	⬆️

... and 87 files with indirect coverage changes

Components	Coverage Δ
Bucket Notification	`75.37% <83.33%> (+59.65%)`	⬆️
Core Library	`75.75% <77.41%> (+14.06%)`	⬆️
Ingestion	`69.24% <ø> (+1.70%)`	⬆️
Lifecycle	`76.14% <ø> (+29.52%)`	⬆️
Oplog Populator	`85.06% <100.00%> (+0.86%)`	⬆️
Replication	`57.79% <100.00%> (+6.39%)`	⬆️
Bucket Scanner	`85.60% <ø> (ø)`

@@                 Coverage Diff                  @@
##           development/9.0    #2599       +/-   ##
====================================================
+ Coverage            54.35%   71.11%   +16.76%     
====================================================
  Files                  201      201               
  Lines                13336    13334        -2     
====================================================
+ Hits                  7249     9483     +2234     
+ Misses                6077     3841     -2236     
  Partials                10       10

Flag	Coverage Δ
api:retry	`9.52% <20.00%> (-0.02%)`	⬇️
api:routes	`9.34% <20.00%> (-0.02%)`	⬇️
bucket-scanner	`85.60% <ø> (ø)`
ingestion	`12.29% <0.00%> (+<0.01%)`	⬆️
lib	`7.36% <0.00%> (+<0.01%)`	⬆️
lifecycle	`18.82% <13.33%> (-0.01%)`	⬇️
notification	`1.07% <17.77%> (-0.02%)`	⬇️
replication	`18.59% <2.22%> (+<0.01%)`	⬆️
unit	`48.14% <71.11%> (+43.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

extensions/oplogPopulator/OplogPopulator.js

francoisferrand · 2024-12-16T11:29:05Z

extensions/utils/LocationStatusStream.js

@@ -96,6 +90,12 @@ class LocationStatusStream {
                return cb();
            });
            return undefined;
+        }).catch(err => {
+            this._log.error('Could not connect to MongoDB', {


we should be able to test this case, just need to pass an invalid url

it is tested here : https://github.com/scality/backbeat/pull/2599/files#diff-f7084912bbc3c3ad747fc166fd4faa2e9557d9a6a7787022bc43ae4f21d238e3R164

lib/util/LocationStatusManager.js

tests/functional/ingestion/IngestionReader.js

extensions/notification/configManager/MongoConfigManager.js

tests/functional/ingestion/IngestionReader.js

Issue: BB-632

Issue: BB-632 Co-authored-by: KillianG <[email protected]> Co-authored-by: benzekrimaha <[email protected]>

…s for sinon stubs

Issue: BB-632

francoisferrand · 2025-01-07T11:17:18Z

.github/workflows/tests.yaml

+  # DEBUG WAIT
+  TMATE_SERVER_HOST: ${{ secrets.TMATE_SERVER_HOST }}
+  TMATE_SERVER_PORT: ${{ secrets.TMATE_SERVER_PORT }}
+  TMATE_SERVER_RSA_FINGERPRINT: ${{ secrets.TMATE_SERVER_RSA_FINGERPRINT }}
+  TMATE_SERVER_ED25519_FINGERPRINT: ${{ secrets.TMATE_SERVER_ED25519_FINGERPRINT }}


this pattern is not very safe, as it exposed the secrets to all steps in the workflow : to avoid this, best to inject the secrets only in the debug-wait step... which means basically inlining the separate action, which does not add any value...

Suggested change

# DEBUG WAIT

TMATE_SERVER_HOST: ${{ secrets.TMATE_SERVER_HOST }}

TMATE_SERVER_PORT: ${{ secrets.TMATE_SERVER_PORT }}

TMATE_SERVER_RSA_FINGERPRINT: ${{ secrets.TMATE_SERVER_RSA_FINGERPRINT }}

TMATE_SERVER_ED25519_FINGERPRINT: ${{ secrets.TMATE_SERVER_ED25519_FINGERPRINT }}

francoisferrand · 2025-01-07T11:18:15Z

.github/workflows/tests.yaml

+    - name: Debug wait
+      uses: ./.github/actions/debug-wait
+      timeout-minutes: 60
+      if: failure() && runner.debug == '1'


Suggested change

- name: Debug wait

uses: ./.github/actions/debug-wait

timeout-minutes: 60

if: failure() && runner.debug == '1'

- name: Debug wait

uses: scality/actions/[email protected]

with:

tmate-server-host: ${{ secrets.TMATE_SERVER_HOST }}

tmate-server-port: ${{ secrets.TMATE_SERVER_PORT }}

tmate-server-rsa-fingerprint: ${{ secrets.TMATE_SERVER_RSA_FINGERPRINT }}

tmate-server-ed25519-fingerprint: ${{ secrets.TMATE_SERVER_ED25519_FINGERPRINT }}

continue-on-error: true

timeout-minutes: 60

if: failure() && runner.debug == '1'

francoisferrand · 2025-01-07T11:18:50Z

extensions/notification/configManager/MongoConfigManager.js

     * Connects to MongoDB using the MongoClientInterface
-     * and retreives the metastore collection
+     * and retrieves the metastore collection


Suggested change

* Connects to MongoDB using the MongoClientInterface

* and retreives the metastore collection

* and retrieves the metastore collection

* Connect to MongoDB using the MongoClientInterface

* and retrieve the metastore collection

francoisferrand · 2025-01-07T11:20:01Z

extensions/notification/configManager/MongoConfigManager.js

+        });
+
+        try {
+            client = await client.connect();


should not replace the client variable : use a different variable or single statement/varialbe (const client = new MongoClient().connect())

francoisferrand · 2025-01-07T11:22:47Z

extensions/notification/configManager/MongoConfigManager.js

+                this._mongoVersion = version;
+                return cb();
+            });
+            return undefined;


merge the return statement and invocation of the last "callback-passing" function : to make it more explicit that the processing goes on asynchronously

Suggested change

return undefined;

return getMongoVersion(...);

francoisferrand · 2025-01-07T11:27:30Z

tests/functional/ingestion/IngestionReader.js

+    `mongodb://${testConfig.queuePopulator.mongo.replicaSetHosts}` +
+    '/db?replicaSet=rs0';
+    const client = new MongoClient(mongoUrl, {});
+    let db;


nit: i think it should be possible to initialize this just one, even if client is not connected...

Suggested change

let db;

const db = client.db('metadata', { ignoreUndefined: true });

(may be left as is though, just sharing)

williamlardier · 2025-01-07T15:05:08Z

lib/api/BackbeatAPI.js

            const mongoUrl = constructConnectionString(mongoConfig);
-            return MongoClient.connect(mongoUrl, {
+            const client = new MongoClient(mongoUrl, {
                replicaSet: mongoConfig.replicaSet,
                useNewUrlParser: true,
                useUnifiedTopology: true,


Suggested change

useUnifiedTopology: true,

Same for all files with this option (mongodb/node-mongodb-native#3792)

extensions/oplogPopulator/OplogPopulator.js

benzekrimaha · 2025-01-10T15:13:55Z

/approve

bert-e · 2025-01-10T15:14:09Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/9.0

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.6

Please check the status of the associated issue BB-632.

Goodbye benzekrimaha.

The following options are set: approve

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch 5 times, most recently from 335ce49 to be4bd02 Compare December 12, 2024 13:49

benzekrimaha marked this pull request as ready for review December 12, 2024 13:51

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch from be4bd02 to 22171f5 Compare December 12, 2024 13:51

francoisferrand reviewed Dec 16, 2024

View reviewed changes

extensions/oplogPopulator/OplogPopulator.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 16, 2024

View reviewed changes

lib/util/LocationStatusManager.js Outdated Show resolved Hide resolved

francoisferrand mentioned this pull request Dec 18, 2024

Improvement/bb 632 bump mongo db driver 6 1 #2596

Closed

francoisferrand reviewed Dec 18, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

extensions/notification/configManager/MongoConfigManager.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 23, 2024

View reviewed changes

extensions/notification/configManager/MongoConfigManager.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 23, 2024

View reviewed changes

extensions/notification/configManager/MongoConfigManager.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 23, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 23, 2024

View reviewed changes

tests/functional/ingestion/IngestionReader.js Outdated Show resolved Hide resolved

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch 3 times, most recently from 65f195d to 42913a5 Compare January 7, 2025 08:52

benzekrimaha and others added 4 commits January 7, 2025 09:53

bump mongo 6.11.0

a986fd7

Issue: BB-632

Update mongodb to use promises instead of callbacks

381b460

Issue: BB-632 Co-authored-by: KillianG <[email protected]> Co-authored-by: benzekrimaha <[email protected]>

Fix bad usage of sinon yields, using resolves instead

ac55ea0

Issue: BB-632 Co-authored-by: KillianG <[email protected]> Co-authored-by: benzekrimaha <[email protected]>

Refactor LocationStatusManager tests to use resolves instead of yield…

3be3404

…s for sinon stubs

benzekrimaha added 5 commits January 7, 2025 09:53

reenable all unit tests testing

f53e2af

Issue: BB-632

Fixup test setup due to mongoClient circular struct

6d854af

Issue: BB-632

Fixup test and coverage improvement

b9916a5

Issue: BB-632

Enable debug logging into CI

d63e500

Issue: BB-632

Fix lint warning

5d52931

Issue: BB-632

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch from 42913a5 to 4f3cb35 Compare January 7, 2025 08:53

fixups post reviews

f1db0df

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch from 8fec511 to f1db0df Compare January 7, 2025 09:01

benzekrimaha requested a review from francoisferrand January 7, 2025 10:29

francoisferrand approved these changes Jan 7, 2025

View reviewed changes

williamlardier reviewed Jan 7, 2025

View reviewed changes

fixups post reviews

287b942

benzekrimaha force-pushed the improvement/BB-632-bump-mongo-db-driver-6 branch from 7a641ec to 287b942 Compare January 9, 2025 15:17

benzekrimaha requested a review from williamlardier January 10, 2025 14:20

williamlardier approved these changes Jan 10, 2025

View reviewed changes

bert-e merged commit 287b942 into development/9.0 Jan 10, 2025
9 checks passed

bert-e deleted the improvement/BB-632-bump-mongo-db-driver-6 branch January 10, 2025 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvement/bb 632 bump mongo db driver 6 #2599

Improvement/bb 632 bump mongo db driver 6 #2599

benzekrimaha commented Dec 10, 2024 •

edited

Loading

bert-e commented Dec 10, 2024

bert-e commented Dec 10, 2024

codecov bot commented Dec 10, 2024 •

edited

Loading

francoisferrand Dec 16, 2024

benzekrimaha Jan 7, 2025

francoisferrand Jan 7, 2025

francoisferrand Jan 7, 2025

francoisferrand Jan 7, 2025

francoisferrand Jan 7, 2025

francoisferrand Jan 7, 2025

francoisferrand Jan 7, 2025

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025

benzekrimaha commented Jan 10, 2025

bert-e commented Jan 10, 2025

	let db;
	const db = client.db('metadata', { ignoreUndefined: true });

Improvement/bb 632 bump mongo db driver 6 #2599

Improvement/bb 632 bump mongo db driver 6 #2599

Conversation

benzekrimaha commented Dec 10, 2024 • edited Loading

bert-e commented Dec 10, 2024

Hello benzekrimaha,

bert-e commented Dec 10, 2024

Waiting for approval

codecov bot commented Dec 10, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benzekrimaha commented Jan 10, 2025

bert-e commented Jan 10, 2025

benzekrimaha commented Dec 10, 2024 •

edited

Loading

codecov bot commented Dec 10, 2024 •

edited

Loading