You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "jackye1995 (via GitHub)" <gi...@apache.org> on 2025/08/23 05:23:44 UTC
[PR] feat(services/s3): Add disable_tagging option and full tagging support [opendal]
jackye1995 opened a new pull request, #6532:
URL: https://github.com/apache/opendal/pull/6532
## Summary
This PR implements comprehensive tagging support for S3 write operations with the ability to disable it when needed, following the pattern used in the Azure object_store implementation.
## Changes
- ✅ Add `disable_tagging` configuration option to `S3Config` and `S3Builder`
- ✅ Add `tags` field to `WriteOptions` and `OpWrite` for specifying object tags
- ✅ Implement tag header generation in `S3Core::insert_metadata_headers` with proper URL encoding
- ✅ Add `write_with_tags` capability to indicate tagging support
- ✅ Include comprehensive unit tests for tagging functionality
## Motivation
Some S3-compatible backends don't support object tagging, or users may want to reduce request overhead when tags aren't needed. This feature provides:
- **Full tagging support** when enabled (default behavior)
- **Option to disable tagging** for compatibility with backends that don't support it
- **Performance optimization** by skipping tag processing when not needed
- **Follows established patterns** from Azure object_store implementation
## Usage
```rust
// Enable tagging (default)
let builder = S3Builder::new()
.bucket("my-bucket");
// Disable tagging for compatibility
let builder = S3Builder::new()
.bucket("my-bucket")
.disable_tagging();
// Write with tags
let mut tags = HashMap::new();
tags.insert("Environment".to_string(), "Production".to_string());
tags.insert("Team".to_string(), "Engineering".to_string());
op.write("path/to/file")
.tags(tags)
.await?;
```
## Implementation Details
- Tags are sent via the `x-amz-tagging` header in URL-encoded format
- Tag keys and values are properly encoded to handle special characters
- When `disable_tagging` is true, no tag processing occurs (zero overhead)
- The `write_with_tags` capability reflects whether tagging is enabled
## Test plan
- [x] Unit test verifies tags are included in headers when tagging is enabled
- [x] Unit test verifies tags are excluded when tagging is disabled
- [x] Proper tag formatting and URL encoding verified
- [x] All existing tests pass
- [x] `cargo check --features services-s3` passes
- [x] `cargo test --features services-s3` passes
🤖 Generated with [Claude Code](https://claude.ai/code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/s3): add S3 tagging write support [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #6532:
URL: https://github.com/apache/opendal/pull/6532#discussion_r2296667847
##########
core/src/services/s3/core.rs:
##########
@@ -343,6 +344,31 @@ impl S3Core {
req = req.header(format!("{X_AMZ_META_PREFIX}{key}"), value)
}
}
+
+ // Set object tagging headers if tagging is enabled and tags are provided.
+ if !self.disable_tagging {
+ if let Some(tags) = args.tags() {
+ if !tags.is_empty() {
+ let tags_string = tags
+ .iter()
+ .map(|(k, v)| {
+ format!(
+ "{}={}",
+ k.replace('&', "%26")
+ .replace('=', "%3D")
+ .replace('+', "%2B"),
+ v.replace('&', "%26")
+ .replace('=', "%3D")
+ .replace('+', "%2B")
Review Comment:
Could you provide the source of this logic? I suppose that S3's docs or other materials define it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/s3): add S3 tagging write support [opendal]
Posted by "jackye1995 (via GitHub)" <gi...@apache.org>.
jackye1995 commented on PR #6532:
URL: https://github.com/apache/opendal/pull/6532#issuecomment-3221971512
Sounds good let me do that first following the process, will revert this as a draft for now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/s3): add S3 tagging write support [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #6532:
URL: https://github.com/apache/opendal/pull/6532#issuecomment-3221103010
Hi, @jackye1995 Thank you for working on this!
Adding a new feature such as writing with tags requires an RFC that considers all possible services. We need to conduct research on at least three major object storage vendors, such as S3, GCS, and Azure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org