ft: enable aws backend object copy w/ versioning [S3C-1053] #957
Conversation
electrachong
force-pushed
the
ft/aws-backend-versioning-copy
branch
from
9ad567e
to
6b65d81
Compare
|
PR has been updated. Reviewers, please be cautious. |
lib/api/objectCopy.js
Outdated
| logger.newRequestLoggerFromSerializedUids( | ||
| log.getSerializedUids())); | ||
| const newDataStoreName = | ||
| Array.isArray(destDataGetInfoArr) && |
There was a problem hiding this comment.
don't we have a cleaner way to access the destination data store name?
There was a problem hiding this comment.
it looks like we make sure destDataGetInfoArray is an array (0cc2541#diff-5ce9bf7b97556522975c2fae1a6944efL283) so I guess we don't need all the checks.
There was a problem hiding this comment.
i meant, don't we have separate metadata of the new data store so we don't have to dig into the location array?
There was a problem hiding this comment.
we store the metadata just before this batch delete step, but we pass the same destDataGetInfoArras the object location to store and services.metadataStoreObject doesn't return the object MD, so we'd have to make a separate call to get the new object MD to get the location directly from the metadata (unless we want to refactor)
https://github.com/scality/S3/pull/957/files#diff-5ce9bf7b97556522975c2fae1a6944efL400
There was a problem hiding this comment.
I do see there is a separate dataStoreName property for object MD but I don't think we set it for the new metadata in objectCopy...
There was a problem hiding this comment.
we should be. not even in @nicolas2bert's new pr?
There was a problem hiding this comment.
I think Nicolas introduces a change in his PR where he gets the controlling dest location constraint which we might be able to use to set the dataStoreName prop appropriately. So far we're only copying an object to a destination location based on the request/source objMd location header (depending on whether we use REPLACE or COPY), I think.
There was a problem hiding this comment.
by so far you mean before Nicolas's pr? The three of us should discuss tomorrow.
lib/data/external/AwsClient.js
Outdated
| @@ -327,6 +329,12 @@ class AwsClient { | |||
| `AWS: ${err.message}`) | |||
| ); | |||
| } | |||
| if (!completeMpuRes.VersionId) { | |||
There was a problem hiding this comment.
This is from the previous commit?
There was a problem hiding this comment.
Yeah, if we merge the other pr first and rebase I think it should disappear from the diff
I left the change there because I need some of the other changes to this file for this PR (the missingVerIdInternalError const)
electrachong
force-pushed
the
ft/aws-backend-versioning-copy
branch
from
0cc2541
to
0c42c69
Compare
|
@ironman-machine try |
|
Hello @electrachong "try": Success: Try build successfully launched on 'http://ci.ironmann.io/gh/scality/Integration/16110' with the following env. args: {
"SCALITY_INTEGRATION_BRANCH": "ultron/master",
"REPO_NAME": "S3",
"DEFAULT_BRANCH": "master",
"SCALITY_S3_BRANCH": "ft/aws-backend-versioning-copy"
} |
|
end to end passed |
| // still send along serverSideEncryption info so algo | ||
| // and masterKeyId stored properly in metadata | ||
| if (sourceIsDestination && storeMetadataParams.locationMatch) { | ||
| if (sourceIsDestination && storeMetadataParams.locationMatch | ||
| && !isVersionedObj) { |
electrachong
force-pushed
the
ft/aws-backend-versioning-copy
branch
from
0c42c69
to
4f1ca9a
Compare
electrachong
changed the title
No description provided.