-
Notifications
You must be signed in to change notification settings - Fork 0
Added headers for CRR Cascaded #24
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,8 @@ namespace cloudserver.backbeatRoutes | |||||
| @http(method: "PUT", uri: "/_/backbeat/metadata/{Bucket}/{Key+}") | ||||||
| operation PutMetadata { | ||||||
| input: PutMetadataInput, | ||||||
| output: PutMetadataOutput | ||||||
| output: PutMetadataOutput, | ||||||
| errors: [MicroVersionIdAlreadyStoredException, StaleMicroVersionIdException] | ||||||
| } | ||||||
|
|
||||||
| structure PutMetadataInput { | ||||||
|
|
@@ -33,12 +34,41 @@ structure PutMetadataInput { | |||||
|
|
||||||
| @httpHeader("X-Scal-Request-Uids") | ||||||
| RequestUids: String, | ||||||
|
|
||||||
|
|
||||||
| /// MicroVersionId of the metadata being written. If specified, call will be rejected if | ||||||
| /// the existing metadata document already has a newer `micro-version-id`: | ||||||
| /// this ensures `micro-version-id` is monotonic, and thus allows handling | ||||||
| /// conflicting writes. If not specified, the update will be performed inconditionnally. | ||||||
| @httpHeader("x-scal-micro-version-id") | ||||||
| MicroVersionId: String, | ||||||
|
|
||||||
| @httpPayload | ||||||
| Body: Blob | ||||||
| } | ||||||
|
|
||||||
| structure PutMetadataOutput { | ||||||
| /// Version ID of the stored metadata | ||||||
| versionId: String | ||||||
| } | ||||||
|
|
||||||
| /// Returned by PutMetadata when the incoming microVersionId matches the one | ||||||
| /// already at the destination. The write is skipped. | ||||||
| @error("client") | ||||||
| @httpError(409) | ||||||
| structure MicroVersionIdAlreadyStoredException { | ||||||
| @required | ||||||
| message: String | ||||||
| } | ||||||
|
|
||||||
| /// Returned by PutMetadata when the incoming microVersionId is older than | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stale is only older, equal is MicroVersionIdAlreadyStoredException |
||||||
| /// the destination's current value (can happen with stale cascade events for example). | ||||||
| /// In that case there is no write. | ||||||
| @error("client") | ||||||
|
francoisferrand marked this conversation as resolved.
|
||||||
| @httpError(409) | ||||||
| structure StaleMicroVersionIdException { | ||||||
|
maeldonn marked this conversation as resolved.
|
||||||
| @required | ||||||
| message: String, | ||||||
|
|
||||||
| @httpHeader("x-scal-micro-version-id") | ||||||
| microVersionId: String | ||||||
| } | ||||||
|
maeldonn marked this conversation as resolved.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "name": "@scality/cloudserverclient", | ||
| "version": "1.0.8", | ||
| "version": "1.0.9", | ||
| "engines": { | ||
| "node": ">=20" | ||
| }, | ||
|
|
||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving for now, what do other ppl think about the naming ?
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.
@scality/raving-robots WDYT?
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.
x-scal-micro-version-idis simpler and easier to understand.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.
🤝ok.
We however should document the semantics of this field here: i.e. it will conflict (and not write metadata) if micro version id is older than the one already stored
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.
@SylvainSenechal please add a comment to explain the semantics of this field
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.
"can be used to detect conflicts on writes" is not the semantics.
The semantics should explain how it behaves: something like "If specified, PutMetadata will be rejected if the existing object version already has a newer micro-version-id"