fix: Only strip config key prefix when present - #755
Draft
kylebarron wants to merge 1 commit into
Draft
Conversation
`IntoPyObject` for the S3/GCS/Azure config keys assumed every key
`as_ref()`s to a prefixed string and used `.expect()` to strip it. But
object_store delegates client and encryption keys to inner enums —
`Self::Client(opt) => opt.as_ref()` — which return unprefixed names like
`allow_http`. Reading `.config` on a store built with any client option
therefore panicked:
S3Store("bucket", allow_http=True).config
PanicException: Expected config prefix to start with aws_
Strip the prefix only when it is actually present, matching what
object_store's own `FromStr` does (`strip_prefix("aws_").unwrap_or(s)`).
Separately, `aws_endpoint_url_s3` is the one key with no unprefixed
alias upstream, so the `endpoint_url_s3` we emit could not be parsed
back in — breaking `S3Store(config=store.config)` and pickling. Retry
the parse with the `aws_` prefix on failure.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
IntoPyObjectfor the S3/GCS/Azure config keys assumed every keyas_ref()s to a prefixed string, and used.expect()to strip it:But object_store delegates client and encryption keys to inner enums —
Self::Client(opt) => opt.as_ref(),Self::Encryption(opt) => opt.as_ref()— which return unprefixed names likeallow_http. So reading.configon a store built with any client option panicked:All three stores were affected (
aws_,google_,azure_).Fix
Strip the prefix only when it's actually present, matching what object_store's own
FromStrdoes (s.strip_prefix("aws_").unwrap_or(s)).Also:
aws_endpoint_url_s3round-tripWhile auditing every config key across the three stores for the same asymmetry,
aws_endpoint_url_s3turned out to be the only key with no unprefixed alias upstream —from_stracceptsaws_endpoint_url_s3but notendpoint_url_s3, unlike its siblingendpoint_url_stswhich has both. Since we emit keys unprefixed, the value we produced couldn't be fed back in:PyAmazonS3ConfigKey::extractnow retries the parse with anaws_prefix on failure. Worth an upstream PR adding the missing alias too.Testing
Regression tests added for each store. I also scanned every config key alias in all three stores for these two failure modes; no others remain.
Note
Found while investigating #753. This does not fix that issue —
AWS_ENDPOINT_URL_S3beating an explicitendpoint=kwarg is a separate problem (EndpointandS3Endpointare distinct upstream keys, andbuild()doesself.s3_endpoint.or(self.endpoint)unconditionally). It does, however, make theaws_endpoint_url_s3=workaround usable with.config/pickling.🤖 Generated with Claude Code