Improvement/Fixup on buckets creations#2371
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development/8.2 #2371 +/- ##
===================================================
+ Coverage 70.05% 70.29% +0.24%
===================================================
Files 218 219 +1
Lines 17443 17528 +85
Branches 3605 3630 +25
===================================================
+ Hits 12219 12321 +102
+ Misses 5220 5203 -17
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
18f1c2f to
9f2d97f
Compare
fcea66d to
22a5462
Compare
| 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, |
There was a problem hiding this comment.
that is very weird:
newBucketMDis the "serializable" variant ofbucketMD: if typing is needed, it should be done inBucketInfo.makeSerializable- adding the type in here will only affect this specific function, as
payloadis only used here - Passing
bucketMD.capabilitiesto 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...
There was a problem hiding this comment.
Actually like for the quotaMax , here we don't really need the makeSerializable to make capabilities typed the way it's needed to be set in mongodb as it's only used here thus in the payload used to create the bucket
49797af to
073daf4
Compare
0864ba7 to
fe130d9
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
fe130d9 to
6916e51
Compare
francoisferrand
left a comment
There was a problem hiding this comment.
Also please update the PR with proper title and description
c3b30cf to
c17ef38
Compare
In some tests, we actually pass the capabilities value to the createBucket function, overriding it to undefined breaks the behavior. This commit fixes that by passing the capabilities value to the createBucket function. Issue: ARSN-492
e00921c to
a8070d0
Compare
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARSN-492. Goodbye benzekrimaha. |
with the migration to typescript the bucket creation has been changed always passing the capabilities as
undefined, which is now breaking s3utils as we’re not able to retrieve the capabilities in order to update themIssue: ARSN-492