Skip to content

Improvement/s3 utils 171 bump deps#336

Merged
bert-e merged 4 commits intodevelopment/1.16from
improvement/S3UTILS-171-bump-deps
May 6, 2025
Merged

Improvement/s3 utils 171 bump deps#336
bert-e merged 4 commits intodevelopment/1.16from
improvement/S3UTILS-171-bump-deps

Conversation

@benzekrimaha
Copy link
Contributor

@benzekrimaha benzekrimaha commented Apr 9, 2025

Issue: S3UTILS-171

@bert-e
Copy link
Contributor

bert-e commented Apr 9, 2025

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.

@benzekrimaha benzekrimaha changed the base branch from development/1.14 to development/1.15 April 9, 2025 10:06
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 3 times, most recently from ad3aed3 to f3c0a03 Compare April 11, 2025 15:23
@scality scality deleted a comment from bert-e Apr 11, 2025
@bert-e
Copy link
Contributor

bert-e commented Apr 11, 2025

Waiting for approval

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

  • the author

  • 2 peers

@benzekrimaha benzekrimaha marked this pull request as ready for review April 11, 2025 15:24
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Adding this one as well: https://github.com/scality/s3utils/actions/runs/14406521365/job/40404317245#step:9:26

We can just replace "ephemeralForTest" with "wiredTiger", it'll just work

@bert-e
Copy link
Contributor

bert-e commented Apr 15, 2025

Incorrect fix version

The Fix Version/s in issue S3UTILS-171 contains:

  • 1.16.0

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

  • 1.15.9

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

@francoisferrand
Copy link
Contributor

this PR should target a new branch, to ensure we don't introduce breaking change

@benzekrimaha benzekrimaha changed the base branch from development/1.15 to development/1.16 April 15, 2025 09:27
@bert-e
Copy link
Contributor

bert-e commented Apr 15, 2025

Waiting for approval

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

  • the author

  • 2 peers

@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch from cace95a to a521e50 Compare April 15, 2025 10:02
if (data.owner !== 'scality') {return;}
const cb = this._getCallback(data.id);
if (!cb) return;
if (!cb) {return;}
Copy link
Contributor

Choose a reason for hiding this comment

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

same


queueSetup() {
this._queue = async.queue(({ bucket, batch, getNext }, done) => {
this._queue = async.queue((task, done) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change? is it not working anymore, or flagged by eslint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not working anymore with the async bump

Copy link
Contributor

Choose a reason for hiding this comment

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

that is weird, should be purely a node "syntax" thing, unrelated to async :-/

listingLimit,
};
// eslint-disable-next-line no-use-before-define
};
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space at EOL

@@ -1,4 +1,4 @@
ARG NODE_VERSION=16.20.2-bullseye-slim
ARG NODE_VERSION=22.14.0-bookworm

Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment, the image includes python, for the scripts in the utapi folder (c.f. https://scality.atlassian.net/browse/S3UTILS-172)

With the upgrade, a newer version of python will be used: so we need to check that the scripts still work (or find a way to remove them and drop python from the image, i.e. move forward with s3utils-172...)

Copy link
Contributor

@francoisferrand francoisferrand Apr 29, 2025

Choose a reason for hiding this comment

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

there is now 22.15.0-bookworm-slim, should we use it?
or just "plan" a global bump from 22.14 to 22.15 ?

@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch from e845d1a to 22877d5 Compare April 16, 2025 16:07
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch from 7c39688 to 2798552 Compare April 18, 2025 12:32
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 2 times, most recently from e3f7843 to 5f111d3 Compare April 22, 2025 13:46
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

You can add a commit to bump if you want 🙂

},
ingestion: null,
},
InternalTestBucketMD: {
Copy link

Choose a reason for hiding this comment

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

Suggested change
InternalTestBucketMD: {
internalTestBucketMD: {

@ghost
Copy link

ghost commented Apr 22, 2025

There is also a unit test failure that requires attention, as it affects a sub dependency...

@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 3 times, most recently from dffe032 to c31aa35 Compare April 28, 2025 07:08
@scality scality deleted a comment from bert-e Apr 29, 2025
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 4 times, most recently from 4590600 to 711deb2 Compare April 30, 2025 16:19
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch from 4871ee0 to a01d856 Compare April 30, 2025 17:03
@benzekrimaha
Copy link
Contributor Author

/approve

@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 2 times, most recently from 1ecf862 to 4513c4d Compare April 30, 2025 17:07
- The async module bump lead to the need of using callbacks on functions like
doWhilst.
- Also we need to add for mongo some parameters to have the expected
return , in this specific case  returnDocument: 'after',
includeResultMetadata: true.
- In our tests, bucketInfos used to send an empty array,
this commit actually uses fill to send the intended array as before
async used to just ignore the empty array and return the expected result.

Issue: S3UTILS-171
@benzekrimaha benzekrimaha force-pushed the improvement/S3UTILS-171-bump-deps branch 2 times, most recently from 254ba0b to ba16cc2 Compare May 6, 2025 10:48
@bert-e
Copy link
Contributor

bert-e commented May 6, 2025

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

  • ✔️ development/1.16

The following branches have NOT changed:

  • development/1.13
  • development/1.14
  • development/1.15
  • development/1.4

Please check the status of the associated issue S3UTILS-171.

Goodbye benzekrimaha.

The following options are set: approve

@bert-e bert-e merged commit ba16cc2 into development/1.16 May 6, 2025
10 checks passed
@bert-e bert-e deleted the improvement/S3UTILS-171-bump-deps branch May 6, 2025 12:40
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.

4 participants