-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Rename S3 Filesystem config socket-read-timeout to socket-timeout #26263
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
base: master
Are you sure you want to change the base?
Rename S3 Filesystem config socket-read-timeout to socket-timeout #26263
Conversation
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
We don't do breaking changes like that. We use @LegacyConfig to redirect previous config property to a new one. |
lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3FileSystemConfig.java
Show resolved
Hide resolved
ec38f88
to
d815533
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
The property `s3.socket-read-timeout` was misleading, as it sets the underlying HTTP client's `socketTimeout`, which controls both read and write timeouts. To avoid confusion and better reflect its actual behavior, the property has been renamed to `s3.socket-timeout`. This change improves clarity for users configuring timeouts in the S3 filesystem and aligns the property name with its actual semantics.
d815533
to
a513434
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Hi @ebyhr can you please help me with the CLA approval? I have 5 other PR waiting for this approval. |
/test-with-secrets sha=a513434853670d70af73ed913ea732839253af57 |
The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/16456523843 |
Description
This PR renames the configuration property
s3.socket-read-timeout
tos3.socket-timeout
in the S3 filesystem connector. The previous propertyname was misleading, as it actually sets the underlying HTTP client's
socketTimeout
, which controls both read and write timeouts. The newproperty name,
s3.socket-timeout
, more accurately reflects its behaviorand improves clarity for users configuring S3 timeouts.
Additional context and related issues
The change updates the property name in both the code and documentation to
ensure consistency and avoid confusion. No functional behavior is changed;
the timeout continues to apply to both read and write operations at the
HTTP client level. This update aligns the property name with its actual
semantics and helps prevent misconfiguration by users.
Release notes
Closes: #26088