-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
bugfix: S3UTILS-110 pass VersionId to PutMetadata route #265
base: development/1.13
Are you sure you want to change the base?
bugfix: S3UTILS-110 pass VersionId to PutMetadata route #265
Conversation
- use ObjectMD model instead of raw parsed JSON object - update the new microVersionId field before putting the metadata changes, to force MongoDB to apply the change and generate an event in the oplog - side fix: call MetadataWrapper.close() so that the script exits after completion (cherry picked from commit 9e97958)
Fix bug on updating metadata. Add 'DEBUG' option to output debug level information. (cherry picked from commit 99a8180)
(cherry picked from commit ac87ae2)
(cherry picked from commit 724008d)
The code used to re-create the replication state, which could break some fields (esp. dataLocation) or remove some replication backends. We now simply update the status, which is safer (more future-proof) and simpler. Issue: S3UTILS-77 (cherry picked from commit 9df7207)
This is especially needed in case the object was created before replication is enabled on the bucket. Issue: S3UTILS-76 (cherry picked from commit ab22723)
Update BackbeatClient with latest schema from BackbeatAPI 8.4 branch
Make sure the `PUT /_/backbeat/metadata` route gets the VersionId attribute, so that it is not confused in thinking we are attempting to update the master version.
Hello jonathan-gramain,My role is to assist you with the merge of this Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
const versions = data.DeleteMarkers | ||
? data.Versions.concat(data.DeleteMarkers) : data.Versions; | ||
return _markPending(bucket, versions, err => { | ||
return _markPending(bucket, data.Versions.concat(data.DeleteMarkers), err => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not concat if data.DeleteMarkers
is null?
e.g. keep the older code or data.Versions.concat(data.DeleteMarkers || [])
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this extra check was introduced in the Zenko version to cope with the change in API (using metadataUtil listing instead of S3), it should not be necessary and 1.4.1 does not have this check, as the SDK always returns an empty array if there is no delete marker.
} = require('async'); | ||
const werelogs = require('werelogs'); | ||
|
||
const { ObjectMD } = require('arsenal').models; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this imports arsenal 8 models, which may not be suitable for S3C ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory they should be compatible, but a few fields have been added in 8 (I see content-language
for example which does not look to be optional but filled with an empty string by default). While they should not cause trouble in practice I agree that we should be careful and it may be better to avoid using model 8 on S3C.
I reverted the crrExistingObjects script to its version 1.4.1 compatible with S3C, then cherry-picked what seems like relevant changes that were applied afterwards on the MongoDB-specific version on top.
On top of this, I then added the actual fix for S3C-6244