Skip to content

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.

Sign up for GitHub

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

Open
SylvainSenechal wants to merge 2 commits into
base: development/1.0
Choose a base branch
from
Open

Added headers for CRR Cascaded #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdedc59
Add new headers for CRR Cascaded
SylvainSenechal May 21, 2026
94c822c
Bump version to 1.0.9
SylvainSenechal May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions models/backbeatRoutes/putMetadata.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ namespace cloudserver.backbeatRoutes
@http(method: "PUT", uri: "/_/backbeat/metadata/{Bucket}/{Key+}")
operation PutMetadata {
input: PutMetadataInput,
output: PutMetadataOutput
output: PutMetadataOutput,
errors: [StaleMicroVersionIdException]
}

structure PutMetadataInput {
Expand Down Expand Up @@ -33,12 +34,31 @@ structure PutMetadataInput {

@httpHeader("X-Scal-Request-Uids")
RequestUids: String,


@httpHeader("x-scal-micro-version-id")
MicroVersionId: String,

@francoisferrand francoisferrand Jun 1, 2026

Copy link
Copy Markdown
Contributor

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)

@SylvainSenechal SylvainSenechal Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

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 ?

@francoisferrand francoisferrand Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scality/raving-robots WDYT?

@maeldonn maeldonn Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x-scal-micro-version-id is simpler and easier to understand.

@francoisferrand francoisferrand Jun 12, 2026

Copy link
Copy Markdown
Contributor

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


@httpPayload
Body: Blob
}

structure PutMetadataOutput {
/// Version ID of the stored metadata
versionId: String
versionId: String,

@maeldonn maeldonn Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align all fields to camelCase or PascalCase

@SylvainSenechal SylvainSenechal Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, versionId is an existing field, i think it was already lowered case even before we started using cloudserverclient.
Looking at many other apis, their output is almost always using lowered case versionIds, so I think instead i will switch ReplicationLoop to camelCase


@httpHeader("x-scal-replication-loop")
replicationLoop: Boolean

@francoisferrand francoisferrand Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when do we have 200 and "replicationLoop" ?
is this not just a StaleMicroVersionIdException as well (with the same micro version id) ?

}

/// Returned by PutMetadata when the incoming microVersionId is older than
/// the destination's current value (stale cascade event). The write is not
/// applied; the caller should mark the replication as FAILED (non-retryable).
@error("client")

@benzekrimaha benzekrimaha Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a test for this changes the generated client contract, but nothing checks that MicroVersionId / versionId are serialized as headers, nor that the new 409 errors are deserialized with their headers

@SylvainSenechal SylvainSenechal Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing cloudserver client, there is some kind of circular dependency where I can test the client either here, in cloudserver or in backbeat.

I have functional tests in both backbeat and cloudserver that triggers these errors

@httpError(409)
structure StaleMicroVersionIdException {

@maeldonn maeldonn Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same doc-comment suggestion as VersionIdCollisionException:

/// Returned by PutMetadata when the incoming microVersionId is older than
/// the destination's current value (stale cascade event). The write is not
/// applied; the caller should mark the replication as FAILED (non-retryable).
@error("client")
@httpError(409)
structure StaleMicroVersionIdException { ... }

@required
message: String,

@httpHeader("x-scal-micro-version-id")
microVersionId: String
}
21 changes: 19 additions & 2 deletions models/backbeatRoutes/putdata.smithy
View file
Open in desktop
Comment thread
maeldonn marked this conversation as resolved.

@SylvainSenechal SylvainSenechal May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Up @@ -7,7 +7,8 @@ use aws.auth#unsignedPayload
@unsignedPayload
operation PutData {
input: PutDataInput,
output: PutDataOutput
output: PutDataOutput,
errors: [VersionIdCollisionException]
}

structure PutDataInput {
Expand All @@ -31,6 +32,9 @@ structure PutDataInput {
@httpHeader("X-Scal-Request-Uids")
RequestUids: String,

@httpHeader("x-scal-version-id")
VersionId: String,

@httpPayload
@default("")
Body: StreamingBlob
Expand All @@ -45,7 +49,20 @@ structure PutDataOutput {

@httpHeader("x-amz-server-side-encryption-customer-algorithm")
SSECustomerAlgorithm: String,

@httpHeader("x-amz-server-side-encryption-aws-kms-key-id")
SSEKMSKeyId: String
}

/// Returned by PutData when the destination already has an object at this
/// versionId. The existing microVersionId is included in the response header
/// so the caller can run the cascade loop/stale/proceed classification.
@error("client")
@httpError(409)
structure VersionIdCollisionException {

@maeldonn maeldonn Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backbeat will pattern-match on this (err.name == VersionIdCollisionException). Consider documenting the error:

/// Returned by PutData when the destination already has an object at this
/// versionId. The existing microVersionId is included in the response header
/// so the caller can run the cascade loop/stale/proceed classification.
@error("client")
@httpError(409)
structure VersionIdCollisionException { ... }

@required
message: String,

@httpHeader("x-scal-micro-version-id")
microVersionId: String
}
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
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"
},
Expand Down
Loading