Skip to content

Add Content_length header to S3 create_multipart #496

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

Jump to bottom
Merged
merged 1 commit into from
Oct 1, 2025
Merged

Add Content_length header to S3 create_multipart #496

merged 1 commit into from
Oct 1, 2025

Conversation

@dreamtalen
Copy link
Contributor

@dreamtalen dreamtalen commented Sep 22, 2025

Which issue does this PR close?

Closes #495

Rationale for this change

Fixes error 411 when using S3 Client's multipart upload on GCS S3-compatible mode

What changes are included in this PR?

Adding the Content-Length header in the create_multipart() request as GCS requires.

Are there any user-facing changes?

User won't see error 411 when using S3 Client's multipart upload on GCS S3-compatible mode

@dreamtalen dreamtalen mentioned this pull request Sep 22, 2025
Closed
@dreamtalen
Copy link
Contributor Author

dreamtalen commented Sep 25, 2025

@tustvold Could you help take a look please?

alamb
alamb reviewed Sep 27, 2025
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution @dreamtalen -- this code looks good to me

Can you please add a test to this code? Maybe just a unit test that shows a create_multipart request has the expected content length? I am worried that we would lose this feature in some future refactor and not know about it

Also, have you tested this code in your own environment and verified it works?

@dreamtalen dreamtalen force-pushed the length-header-create-mpu branch from 8f6ec21 to f9ec38a Compare October 1, 2025 06:17
@dreamtalen
Copy link
Contributor Author

dreamtalen commented Oct 1, 2025

Thanks for this contribution @dreamtalen -- this code looks good to me

Can you please add a test to this code? Maybe just a unit test that shows a create_multipart request has the expected content length? I am worried that we would lose this feature in some future refactor and not know about it

Also, have you tested this code in your own environment and verified it works?

Thanks for the review. A unit test is added and I verified this change makes MPU work correctly for both GCS and AWS S3 with the S3Client.

@dreamtalen dreamtalen force-pushed the length-header-create-mpu branch from f9ec38a to 0237e32 Compare October 1, 2025 16:54
alamb
alamb approved these changes Oct 1, 2025
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @dreamtalen 🙏

@alamb alamb merged commit 4102e4d into apache:main Oct 1, 2025
8 checks passed
Simon-3008-Simon added a commit to sap-contributions/arrow-rs-object-store that referenced this pull request Oct 14, 2025
@Simon-3008-Simon @alamb
@mbrobbel @dependabot @kylebarron @philjb @asubiotto @matthewmturner @Friede80 @dreamtalen
FOS-11081: sync code (#3)
c40000b 
* Improve documentation for http client timeout (apache#390)

* chore: fix some clippy 1.89 warnings and ignore some doctests on wasm32 (apache#468)

* chore: fix some clippy 1.89 warnings

* fix another warning

* Skip some doctests for wasm32

* build(deps): bump actions/checkout from 4 to 5 (apache#463)

Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Allow "application_credentials" in `impl FromStr for GoogleConfigKey` (apache#467)

* build(deps): bump actions/setup-python from 5 to 6 (apache#476)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5 to 6.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v5...v6)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump actions/setup-node from 4 to 5 (apache#477)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 4 to 5.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v4...v5)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump actions/github-script from 7 to 8 (apache#478)

Bumps [actions/github-script](https://github.com/actions/github-script) from 7 to 8.
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](actions/github-script@v7...v8)

---
updated-dependencies:
- dependency-name: actions/github-script
  dependency-version: '8'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(client/retry): include error info in logs when retry occurs (apache#487)

On a request retry, it logs an info message stating that an error was
encountered and information about the retry process but it hasn't
included any details about the error that is causing the retry. This PR
updates the logging to include the status if it is a server error and
the http error kind if a transport error occurred. While the last error
when retries are exhausted is returned up the call stack, the
intermediate errors need not be exactly the same. It is helpful to
include some minimum information about what error triggered a retry each
time it happens.

* aws: downgrade credential provider info! log messages to debug! (apache#436)

These log messages are very noisy.

* Add storage class for aws, gcp, and azure (apache#456)

* Add storage class for aws and gcp

* Add azure storage class attribute

* Update attribute docs

* Update http client

* Add version 0.12.4 release plan to README (apache#490)

* Fix for clippy 1.90 (apache#492)

* AWS S3: Support STS endpoint, WebIdentity, RoleArn, RoleSession configuration (apache#480)

* Allow setting STS endpoint via env var

* Properly use AmazonS3Builder::credentials_from_env for AssumeRoleWithWebIdentity auth flow

---------

Co-authored-by: Andrew Lamb <[email protected]>

* Revert "refactor: remove AWS dynamo integration (apache#407)" (apache#493)

This reverts commit 034733f.

* Update version to 0.12.4 and add changelog (apache#491)

* Update version to 0.12.4

* Update update_changelog.sh script

* Update changelog

* Last touchups

* Update changelog

* Reapply "refactor: remove AWS dynamo integration (apache#407)" (apache#494)

This reverts commit bebd53b.

* Add Content_length header to S3 create_multipart (apache#496)

* minor: Fix MSRV CI workflow (apache#502)

* ci: Fix MSRV CI workflow

* Update .github/workflows/rust.yml

* Update .github/workflows/rust.yml

---------

Co-authored-by: Matthijs Brobbel <[email protected]>

* All changes to support SAP HDLFS

* add hdlfs

* FOS-9911: support delta path version 2.0

* FOS-10349:  fix object_url (#2)

* FOS-10349: fix object_url

* FOS-11081: sync code

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
Co-authored-by: Matthijs Brobbel <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kyle Barron <[email protected]>
Co-authored-by: Phil Bracikowski <[email protected]>
Co-authored-by: Alfonso Subiotto Marqués <[email protected]>
Co-authored-by: Matthew Turner <[email protected]>
Co-authored-by: Matt Friede <[email protected]>
Co-authored-by: Yongming Ding <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alamb alamb alamb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error 411 (Length Required) when using Multipart PUT on GCP with S3Store

2 participants

@dreamtalen @alamb