add custom_headers parameter to initiate_multipart_upload#446
add custom_headers parameter to initiate_multipart_upload#446ZzIsGod1019 wants to merge 3 commits intodurch:masterfrom
Conversation
fix: command PutBucketLifecycle failed(SignatureDoesNotMatch)
📝 WalkthroughWalkthroughThe PR adds support for custom HTTP headers in multipart upload initiation flows. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
s3/src/bucket.rs (2)
1857-1877: Breaking API change: Consider backward compatibility.Adding
custom_headers: Option<HeaderMap>as a required parameter to the publicinitiate_multipart_uploadmethod is a breaking change for existing callers. Consider whether this warrants a semver major version bump.Additionally, as per coding guidelines, public APIs should include documentation examples. This method currently lacks doc comments demonstrating the new
custom_headersparameter usage.Example documentation to add
/// Initiate multipart upload to s3. /// /// # Example: /// /// ```no_run /// use s3::bucket::Bucket; /// use s3::creds::Credentials; /// use http::HeaderMap; /// use http::header::HeaderName; /// use anyhow::Result; /// /// # #[tokio::main] /// # async fn main() -> Result<()> { /// let bucket = Bucket::new("my-bucket", "us-east-1".parse()?, Credentials::default()?)?; /// /// // Without custom headers /// let response = bucket.initiate_multipart_upload("/path/to/file", "application/octet-stream", None).await?; /// /// // With custom headers (e.g., Cache-Control, x-amz-meta-*) /// let mut headers = HeaderMap::new(); /// headers.insert( /// HeaderName::from_static("cache-control"), /// "public, max-age=3600".parse().unwrap(), /// ); /// let response = bucket.initiate_multipart_upload("/path/to/file", "application/octet-stream", Some(headers)).await?; /// # Ok(()) /// # } /// ``` #[maybe_async::async_impl] pub async fn initiate_multipart_upload(
1802-1809: Inconsistency: Sync streaming path doesn't support custom headers.The sync
_put_object_stream_with_content_typemethod always passesNoneforcustom_headers, while the async path (_put_object_stream_with_content_type_and_headers) properly propagates custom headers. Consider adding a sync equivalent that supports custom headers for consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
s3/src/bucket.rss3/src/command.rss3/src/request/request_trait.rs
🧰 Additional context used
📓 Path-based instructions (2)
s3/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use existing error types from
s3/src/error.rsrather than defining new error types
Files:
s3/src/command.rss3/src/request/request_trait.rss3/src/bucket.rs
{s3,aws-region,aws-creds}/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
{s3,aws-region,aws-creds}/src/**/*.rs: Follow the async/sync abstraction pattern usingmaybe_asyncmacros for runtime-agnostic implementations
Mark integration tests with#[ignore]if they require AWS credentials
Provide documentation examples for all public APIs
Files:
s3/src/command.rss3/src/request/request_trait.rss3/src/bucket.rs
🧬 Code graph analysis (1)
s3/src/request/request_trait.rs (2)
s3/src/signing.rs (1)
headers(141-144)s3/src/request/tokio_backend.rs (1)
headers(124-135)
🔇 Additional comments (6)
s3/src/command.rs (2)
135-138: LGTM! Consistent with existing patterns.The addition of
custom_headers: Option<HeaderMap>toInitiateMultipartUploadmirrors the existing pattern inPutObjectandPresignPutvariants, providing a consistent API surface.
262-262: LGTM! Correct pattern matching update.Using
..to ignore the newcustom_headersfield while extractingcontent_typeis the idiomatic approach.s3/src/request/request_trait.rs (1)
699-706: LGTM! Header merging correctly follows the established pattern.The custom headers for
InitiateMultipartUploadare merged using the same approach asPutObject(lines 691-697). Headers are inserted before the host header and authorization calculation, ensuring they are properly included in the AWS signature.s3/src/bucket.rs (3)
1864-1867: LGTM! Command construction correctly includes custom_headers.The
Command::InitiateMultipartUploadis properly constructed with bothcontent_typeandcustom_headersfields.
1879-1899: Sync implementation mirrors async correctly.The sync variant properly accepts and passes
custom_headersto the command, maintaining consistency with the async implementation.
1667-1668: LGTM! Custom headers correctly propagated in async multipart flow.The
custom_headersparameter is properly forwarded toinitiate_multipart_upload, enabling per-request header customization for multipart uploads.
This PR adds support for custom HTTP headers in the
initiate_multipart_uploadmethod, allowing users to pass custom headers (such as cache-control, metadata, storage class, etc.) when initiating multipart uploads to S3.This feature aligns the
initiate_multipart_uploadAPI with the existingput_object_with_headersfunctionality, providing a consistent interface for setting custom headers across different upload methods.This change is
Summary by CodeRabbit
New Features
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.