Skip to content

fix: preserve explicit provider options for matching implicit profiles#20

Open
saschabuehrle wants to merge 1 commit intoNVIDIA:mainfrom
saschabuehrle:fix/issue-19-config-options
Open

fix: preserve explicit provider options for matching implicit profiles#20
saschabuehrle wants to merge 1 commit intoNVIDIA:mainfrom
saschabuehrle:fix/issue-19-config-options

Conversation

@saschabuehrle
Copy link

Bug

Fixes #19 — when resolve_storage_client() uses an implicit profile like _s3-my-bucket, StorageClientConfig.from_file() rebuilt the profile with only base_path, so explicit options (for example endpoint_url) were dropped.

Fix

  • In implicit profile resolution, detect when a matching explicit profile (s3-my-bucket) already exists with the same provider type and base_path.
  • Reuse that explicit profile config (copied under the implicit profile name) so provider options are preserved.
  • Keep fallback behavior unchanged for implicit profiles without a matching explicit profile.
  • Added a focused regression test for the matching-profile path.

Testing

  • uv run pytest -q tests/test_multistorageclient/unit/test_implicit_profile_config.py

Happy to address any feedback.

Greetings, saschabuehrle

@saschabuehrle saschabuehrle requested a review from a team March 19, 2026 17:13
@commiterate
Copy link
Contributor

commiterate commented Mar 19, 2026

There's currently a path_mapping option in the MSC configuration which lets users explicitly map s3://{bucket}/ URLs to msc://{profile}/ URLs (see #19 (comment)).

That lets users explicitly choose which MSC profile to use for S3 URLs instead of choosing the first matching profile in the configuration.

Tie breaking based on appearance order can be difficult to reason about if using the MSC configuration merging feature.

@saschabuehrle
Copy link
Author

Thanks for the pointer on — agreed, that setting is the right option when explicit remaps are needed.\n\nThe intent of this patch is narrower: keep explicitly provided provider options intact when profile auto-resolution still lands on the same implicit provider profile. That way existing explicit endpoint/region overrides aren't dropped unexpectedly.\n\nIf you'd like, I can add a short note in docs clarifying when to use vs profile option overrides.\n\nGreetings, saschabuehrle

@saschabuehrle
Copy link
Author

Small correction to my previous comment (shell formatting ate the inline code style):

Thanks for the pointer on path_mapping — agreed, that setting is the right option when explicit remaps are needed.

The intent of this patch is narrower: keep explicitly provided provider options intact when profile auto-resolution still lands on the same implicit provider profile. That way existing explicit endpoint/region overrides aren't dropped unexpectedly.

If you'd like, I can add a short docs note clarifying when to use path_mapping vs profile option overrides.

Greetings, saschabuehrle

@commiterate
Copy link
Contributor

commiterate commented Mar 20, 2026

Implicit profile names are an internal detail that's subject to change. There's no guarantee that these profiles will always be named _{protocol}-{bucket}.

That makes automatic merging with a user-defined profile named {protocol}-{bucket} (i.e. without the leading underscore) a shaky proposition since it's reaching into unstable internals.

path_mapping on the other hand is a stable public interface and user-defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StorageClientConfig.from_file ignores all options in implicit profile creation

2 participants