Skip to content

[Python] Make --aws_* options defaults and --s3_* options S3 overrides - #40532

Merged
shunping merged 2 commits into
apache:masterfrom
shunping:secret-manager-7
Oct 13, 2026
Merged

shunping merged 2 commits into
apache:masterfrom
shunping:secret-manager-7

Conversation

@shunping

@shunping shunping commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #40529, Part of #40528.

Motivation

#40529 added AwsOptions and deprecated S3Options, so every --s3_* option became a deprecated alias of an --aws_* option. That assumes one set of client settings fits every AWS service, which doesn't hold in practice:

  • Endpoints are often per service. S3-compatible stores (MinIO, Ceph, R2), VPC interface endpoints and FIPS endpoints only serve one service. With a single --aws_endpoint_url=http://minio:9000, any non-S3 client (e.g. Secrets Manager) would be sent to MinIO too.
  • verify / SSL usually follow the endpoint, e.g. a self-signed certificate on an S3-compatible store.
  • The API version is always per service. boto3's api_version picks a service-specific model (S3: 2006-03-01, Secrets Manager: 2017-10-17), and passing one service's version to another fails:
    boto3.client('secretsmanager', api_version='2006-03-01')
    -> DataNotFoundError: Unable to load data for: secretsmanager/2006-03-01/service-2
    

The AWS SDKs handle this with global defaults plus overrides per service, e.g. AWS_ENDPOINT_URL and AWS_ENDPOINT_URL_S3, where the service-specific value wins. This PR adopts the same model.

Changes

  • AwsOptions (--aws_*) sets the defaults for every AWS client: access_key_id, secret_access_key, session_token, endpoint_url, region_name, verify, disable_ssl.
    • --aws_api_version is removed, since a single value can't apply to all services.
    • The deprecated --s3_* aliases and the fallback logic are removed.
  • New AwsOptions.get_client_settings(service_options=None) returns the effective client settings for a service:
    • It starts from the --aws_* defaults and applies overrides from a service options class.
    • A service options class opts in by setting AWS_CLIENT_SETTINGS_PREFIX (e.g. 's3') and defining --<prefix>_<setting> options. An override applies when it's set.
    • Adding options for another service (e.g. Secrets Manager) later only needs a new options class. AwsOptions doesn't change.
  • S3Options (--s3_*) is no longer deprecated:
    • --s3_access_key_id, --s3_secret_access_key, --s3_session_token, --s3_endpoint_url, --s3_region_name, --s3_verify and --s3_disable_ssl override the matching --aws_* defaults for the S3 client only.
    • --s3_api_version is S3-only.
  • S3 client: boto3_client.Client builds the client from aws_options.get_client_settings(s3_options) plus s3_options.s3_api_version, for both PipelineOptions and dict options.
  • CHANGES.md: removed the S3Options deprecation entry, and reworded the AwsOptions entry: --aws_* set defaults for all AWS clients, and --s3_* override them for S3.

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.73%. Comparing base (76f6e6d) to head (3b4809f).
⚠️ Report is 139 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##             master   #40532       +/-   ##
=============================================
+ Coverage     56.16%   79.73%   +23.56%     
=============================================
  Files          1124      487      -637     
  Lines        178336    82278    -96058     
  Branches       1489        0     -1489     
=============================================
- Hits         100170    65606    -34564     
+ Misses        75645    16672    -58973     
+ Partials       2521        0     -2521     
Flag Coverage Δ
go ?
java ?
python 79.73% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shunping shunping changed the title [Python] Resolve AWS credentials from a secret manager via --aws_credentials_secret_manager [Python] Keep --s3_api_version in S3Options and remove --aws_api_version Oct 12, 2026
@shunping
shunping marked this pull request as ready for review October 12, 2026 20:01
@shunping
shunping requested a review from Abacn October 12, 2026 20:01
@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @claudevdm for label python.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@github-actions
github-actions Bot requested a review from claudevdm October 12, 2026 21:13
@shunping shunping changed the title [Python] Keep --s3_api_version in S3Options and remove --aws_api_version [Python] Make --aws_* options defaults and --s3_* options S3 overrides Oct 12, 2026
@shunping
shunping merged commit 3ec3db0 into apache:master Oct 13, 2026
87 of 88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants