Skip to content

Improvement/arsn 492 #2371

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: development/8.2
Choose a base branch
from
Open

Improvement/arsn 492 #2371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2acae9
capabilites value passing instead of undefined in createBucket
benzekrimaha May 6, 2025
22a5462
bump project version
benzekrimaha May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/storage/metadata/mongoclient/MongoClientInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ class MongoClientInterface {
value: {
...newBucketMD,
quotaMax: new Long(newBucketMD.quotaMax || '0'),
capabilities: undefined,
// We're passing the capabilities value here as it's used for testing purposes
capabilities: newBucketMD.capabilities as CapabilitiesMongoDB || undefined,
Copy link
Contributor

@francoisferrand francoisferrand May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

that is very weird:

  • newBucketMD is the "serializable" variant of bucketMD: if typing is needed, it should be done in BucketInfo.makeSerializable
  • adding the type in here will only affect this specific function, as payload is only used here
  • Passing bucketMD.capabilities to createBucket() is not just for testing: today, maybe we do not do it "in prod" and only do it in tests, but since it is part of bucketInfo it should be supported, like all other fields... → if a fix is needed, it is not just for tests...

},
vFormat: this.defaultBucketKeyFormat,
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=20"
},
"version": "8.2.13",
"version": "8.2.14",
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure we want to release just yet, we got a few more that also need to reach 9.0 very soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will actually need to bump, for the s3utils PR

Copy link
Contributor

Choose a reason for hiding this comment

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

yes we need to bump, but not on every PR. We currently have 3 PRs open in arsenal, all of which need to be bump'ed : so better to merge as many PRs as possible, and release/bump just once, after the last one of the batch.

"description": "Common utilities for the S3 project components",
"main": "build/index.js",
"repository": {
Expand Down
Loading