Added headers for CRR Cascaded#24
Conversation
Hello sylvainsenechal,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
There was a problem hiding this comment.
To document errors:
/// Returned when a version of an object already exists
@error("client")
@httpError(409)
structure ConflictException {
@required
message: String
}
@http(method: "PUT", uri: "/_/backbeat/data/{Bucket}/{Key+}?v2")
@unsignedPayload
operation PutData {
input: PutDataInput,
output: PutDataOutput,
errors: [ConflictException]
}e2a81bf to
798a647
Compare
There was a problem hiding this comment.
First I added some tests in this codebase, but they were too functional and not really oriented toward just testing the client itself. + we already have test testing the client itself for putData/putMetadata
So I added the tests in Cloudserver : They are functional tests using CloudserverClient with the micro version id, testing 409 error, actual api behavior etc. I think they are better in cloudserver than in here
| RequestUids: String, | ||
|
|
||
| @httpHeader("x-scal-micro-version-id") | ||
| MicroVersionId: String, |
There was a problem hiding this comment.
as discussed (late) in the design, not an input parameter : PutData should minimize work/logic related to metadata, and should simply return the current microVersionId (instead of "x-scal-cascade-loop-detected")
this will reduce coupling, while giving all flexibility for backbeat to take all decisions.
| @httpHeader("x-scal-cascade-loop-detected") | ||
| CascadeLoopDetected: Boolean |
There was a problem hiding this comment.
PutMetadata is a generic api, used in many more cases than CRR.
→ api should stay generic, no reason to introduce "crr cascaded" vocabulary : stick to some generic concept, like "conflict" or "already have microVersionId"
There was a problem hiding this comment.
I thought about this but I thought it would be better for future maintainers to understand what this field is used for.
I dont wanna go with conflict as this would already be used for microVersionId that are too old, but I'll find something for the equality around "already have micro version id"
There was a problem hiding this comment.
edit: name updated, let me know if you have other name ideas
|
|
||
|
|
||
| @httpHeader("x-scal-micro-version-id") | ||
| MicroVersionId: String, |
There was a problem hiding this comment.
nit: to make the semantic explicit, field could be name something like IfMicroVersionIdOlderThan (i.e. semantics is part of the API)
There was a problem hiding this comment.
Leaving for now, what do other ppl think about the naming ?
798a647 to
a072957
Compare
ISSUE : CLDSRVCLT-14
Crr cascaded design : https://github.com/scality/citadel/pull/349
Related PRs :
Arsenal : scality/Arsenal#2628
Cloudserver : scality/cloudserver#6179
Backbeat : Not done yet
S3utils : scality/s3utils#395