fix: expand env_var substitution in profile option values#20
Open
theiotidiot wants to merge 1 commit into
Open
fix: expand env_var substitution in profile option values#20theiotidiot wants to merge 1 commit into
theiotidiot wants to merge 1 commit into
Conversation
The ADBC connection-profile spec specifies that profile option values containing
`{{ env_var(NAME) }}` are expanded against the process environment when the
profile is loaded. `adbc_driver_manager` performs this expansion when callers
go through its `DriverLocator::Profile` path (via `process_profile_value`),
but databow's `initialize_profile_connection` loads the profile through
`FilesystemProfileProvider` directly and forwards the resulting `OptionValue`s
to the driver without applying the substitution. Profiles that reference env
vars therefore fail at connect time because the literal template string reaches
the driver.
Apply `process_profile_value` to every `OptionValue::String` returned by the
profile, matching the driver manager's own behavior. Add a regression test
that asserts the expanded value reaches the driver (the test points DuckDB at
a database file whose name comes from an env var and verifies the substituted
path exists on disk while the literal-template path does not).
Fixes columnar-tech#19
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.
Summary
When connecting via
databow --profile NAME, option values that use the ADBC{{ env_var(NAME) }}substitution template are forwarded to the driver as literal strings instead of being expanded against the process environment. Profiles that lean on env-var substitution to inject credentials or URIs (per the ADBC connection-profile spec) therefore fail at connect time.adbc_driver_managerperforms this substitution along itsDriverLocator::Profilepath (viaprocess_profile_value), butinitialize_profile_connectionloads the profile throughFilesystemProfileProviderdirectly and skips that step. This patch appliesprocess_profile_valueto everyOptionValue::Stringreturned by the profile, matching the driver manager's own behavior.Test plan
test_profile_env_var_substitutionintegration test that points DuckDB at a database file whose name comes from an env var and asserts the substituted path exists on disk while the literal-template path does not — fails onmain, passes with this change.cargo fmtcargo test— 16 passed.uricontains{{ env_var(...) }}for the auth token.Fixes #19