Bugfix/s3 utils 239 fix count items bucket logging#394
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/1.16 #394 +/- ##
====================================================
+ Coverage 45.23% 45.28% +0.04%
====================================================
Files 85 85
Lines 6046 6049 +3
Branches 1282 1283 +1
====================================================
+ Hits 2735 2739 +4
+ Misses 3265 3264 -1
Partials 46 46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| const { deserializeBigInts, serializeBigInts } = require('./utils/utils'); | ||
|
|
||
| const PENSIEVE = 'PENSIEVE'; | ||
| const unsupportedSerializedBucketInfoFields = [ |
There was a problem hiding this comment.
could have kept the comment about why we do this 🤔
maeldonn
left a comment
There was a problem hiding this comment.
Once normalizeBucketInfoForCountItems updated, it will be good for me !
francoisferrand
left a comment
There was a problem hiding this comment.
Current change introduces a performance impact, which is not worth as already pointed out (esp. considering count items is end of life!)
ce5c1f7 to
37da7d2
Compare
37da7d2 to
9a5c496
Compare
francoisferrand
left a comment
There was a problem hiding this comment.
LGTM.
Do we need the fix on 1.16? I see zenko 2.14 and 2.15 both use s3utils 1.17, and not sure if the problem can be reproduced with zenko 2.13 (which uses cloudserver 9.1.14)
- if not needed → rebase on 1.17
- if needed → add a bump commit on waterflow branch & release both 1.16 and 1.17
In the PR we are nullifying _bucketLoggingStatus which is safe for count-items specifically: that this metadata is unrelated to object count / storage usage computation and only exists on the BucketInfo object because it is part of the full bucket metadata model.
The implementation we now have is cleaner and makes this explicit by naming the fields as unsupported serialized BucketInfo fields for count-items.
The rationale being that : class-backed fields that count-items does not consume should not be rehydrated in the worker path just to count object metadata.
Issue: S3UTILS-239