[dagster-dbt] Honor DBT_PROFILE/DBT_TARGET in the in-process adapter#33937
[dagster-dbt] Honor DBT_PROFILE/DBT_TARGET in the in-process adapter#33937Vamsi-klu wants to merge 1 commit into
Conversation
The dbt subprocess spawned by DbtCliResource inherits os.environ, so dbt's CLI layer reads DBT_PROFILE / DBT_TARGET and selects that profile/target. The in-process adapter used for metadata calls (e.g. column metadata), however, called load_profile with self.profile / self.target only, which are None unless set explicitly on the resource. As a result the two paths could resolve different profiles/targets (e.g. subprocess -> prod, metadata adapter -> the profiles.yml default), causing wrong-credential errors. Fall back to the DBT_PROFILE / DBT_TARGET environment variables when the resource does not set them explicitly, matching dbt's own CLI resolution. Explicit resource configuration still takes precedence. Fixes dagster-io#33818
Greptile SummaryThis PR fixes a parity gap in
Confidence Score: 5/5Safe to merge — the change is two lines of fallback reads from the environment, explicit resource config continues to take precedence, and the fix is scoped to the single in-process code path that was inconsistent. The production change is minimal and correct: it reads two well-known dbt env vars as fallbacks when the resource fields are unset, mirroring what the subprocess path already receives for free. Explicit configuration still wins. No state is mutated, no new dependencies are introduced, and os.getenv returns None (matching the prior default) when the vars are absent. No files require special attention. The test depends on dbt's internal set_invocation_context/set_from_args/get_flags succeeding without mocks, but that is a pre-existing pattern in the test suite and not a correctness risk for the production change.
|
| Filename | Overview |
|---|---|
| python_modules/libraries/dagster-dbt/dagster_dbt/core/resource.py | Two-line fix: reads DBT_PROFILE/DBT_TARGET from the environment as fallbacks when the resource fields are None, then passes them to load_profile. Logic is correct for the common case; os was already imported at module level. |
| python_modules/libraries/dagster-dbt/dagster_dbt_tests/core/test_resource.py | Adds a parametrized test covering env-var honored, explicit-arg-wins, and no-override scenarios. Patches dbt.config.runtime.load_profile correctly (local import inside function body makes the module-level patch work). Test depends on pre-load_profile dbt calls (set_invocation_context, set_from_args, get_flags) succeeding without mocks. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User Code
participant Res as DbtCliResource
participant Env as os.environ
participant LP as dbt load_profile
Note over User,LP: Subprocess path (pre-existing)
User->>Res: .cli(["dbt", "run"])
Res->>Env: subprocess inherits os.environ
Env-->>LP: DBT_PROFILE / DBT_TARGET via Click CLI layer
Note over User,LP: In-process adapter path (this PR)
User->>Res: _initialize_dbt_core_adapter(args)
Res->>Res: "profile_override = self.profile or os.getenv("DBT_PROFILE")"
Res->>Res: "target_override = self.target or os.getenv("DBT_TARGET")"
Res->>LP: load_profile(project_dir, cli_vars, profile_override, target_override)
LP-->>Res: profile object (same warehouse as subprocess)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User as User Code
participant Res as DbtCliResource
participant Env as os.environ
participant LP as dbt load_profile
Note over User,LP: Subprocess path (pre-existing)
User->>Res: .cli(["dbt", "run"])
Res->>Env: subprocess inherits os.environ
Env-->>LP: DBT_PROFILE / DBT_TARGET via Click CLI layer
Note over User,LP: In-process adapter path (this PR)
User->>Res: _initialize_dbt_core_adapter(args)
Res->>Res: "profile_override = self.profile or os.getenv("DBT_PROFILE")"
Res->>Res: "target_override = self.target or os.getenv("DBT_TARGET")"
Res->>LP: load_profile(project_dir, cli_vars, profile_override, target_override)
LP-->>Res: profile object (same warehouse as subprocess)
Reviews (2): Last reviewed commit: "[dagster-dbt] Honor DBT_PROFILE/DBT_TARG..." | Re-trigger Greptile
| profile_override = self.profile or os.getenv("DBT_PROFILE") | ||
| target_override = self.target or os.getenv("DBT_TARGET") |
There was a problem hiding this comment.
Using
or for the fallback means an explicitly-set empty string (profile="") would be treated as falsy and the env-var would win instead of the explicit value. While pydantic doesn't enforce non-empty strings on these fields, a more defensive check makes the intent unambiguous and matches how other dbt tooling handles Optional[str] overrides.
| profile_override = self.profile or os.getenv("DBT_PROFILE") | |
| target_override = self.target or os.getenv("DBT_TARGET") | |
| profile_override = self.profile if self.profile is not None else os.getenv("DBT_PROFILE") | |
| target_override = self.target if self.target is not None else os.getenv("DBT_TARGET") |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| class _StopAdapterInit(Exception): | ||
| """Raised to short-circuit adapter init right after profile resolution.""" | ||
|
|
||
| monkeypatch.delenv("DBT_PROFILE", raising=False) | ||
| monkeypatch.delenv("DBT_TARGET", raising=False) | ||
| for key, value in env.items(): | ||
| monkeypatch.setenv(key, value) | ||
|
|
There was a problem hiding this comment.
Test may not reach the mocked
load_profile
Several real dbt calls run inside _initialize_dbt_core_adapter before load_profile is invoked — specifically set_invocation_context, set_from_args, and get_flags. These are not mocked, so if dbt's internal state isn't clean between parametrize runs (e.g. flags or adapter registry left from a previous test), one of those earlier calls could raise an unexpected exception. pytest.raises(_StopAdapterInit) would not catch it, causing a confusing failure that looks unrelated to the env-var resolution being tested. Mocking or resetting dbt's flags/adapter state around the test body would make the isolation tighter.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0c60a81f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| profile_override = self.profile or os.getenv("DBT_PROFILE") | ||
| target_override = self.target or os.getenv("DBT_TARGET") |
There was a problem hiding this comment.
Gate DBT env overrides by dbt version
In dbt-core 1.7.x, which validate_dbt_version still accepts, the dbt CLI options for --profile and --target do not read DBT_PROFILE / DBT_TARGET (those envvars were added in dbt 1.8), so the subprocess will keep using the project/default profile while these lines force the in-process adapter to use the environment override. In a supported 1.7 deployment with DBT_TARGET=prod set, metadata calls can now connect to a different target than the actual dbt invocation; only apply this fallback for dbt versions whose CLI honors the same envvars.
Useful? React with 👍 / 👎.
| profile_override = self.profile or os.getenv("DBT_PROFILE") | ||
| target_override = self.target or os.getenv("DBT_TARGET") |
There was a problem hiding this comment.
Respect CLI flags before env fallback
When a caller supplies --target or --profile in args while DBT_TARGET / DBT_PROFILE is also set, the subprocess receives those explicit flags via full_dbt_args and dbt/Click gives them precedence over envvars, but the in-process adapter never parses args and instead selects the env override here whenever the resource fields are unset. In that scenario, metadata fetching can connect to the env target/profile while the actual dbt run uses the CLI-specified one; parse the CLI flags before falling back to the environment.
Useful? React with 👍 / 👎.
| # env vars when the resource does not set profile/target explicitly so both | ||
| # paths resolve the same profile. Explicit resource config still wins. | ||
| # Fixes https://github.com/dagster-io/dagster/issues/33818 | ||
| profile_override = self.profile or os.getenv("DBT_PROFILE") |
There was a problem hiding this comment.
Honor DBT_PROFILES_DIR with DBT_PROFILE
When DBT_PROFILE is set together with DBT_PROFILES_DIR and the resource has no explicit profiles_dir, the subprocess inherits DBT_PROFILES_DIR, but _initialize_dbt_core_adapter still sets dbt flags to self.project_dir before calling load_profile. This new fallback can therefore look up the env-selected profile in the wrong profiles directory, either failing adapter initialization or using same-named credentials from the project directory while the actual dbt run uses the env profiles directory.
Useful? React with 👍 / 👎.
|
This is ready for review. It's a one-line fix in For transparency: I could not run the dbt test suite locally (a Drafted-by: Claude Code (Opus 4.8) (no human review before posting) |
b0c60a8 to
8a184cb
Compare
Summary & Motivation
Fixes #33818.
DbtCliResourceresolves the dbt profile/target in two different places:dbt run,dbt build, …): the subprocess inheritsos.environ, so dbt's own CLI layer readsDBT_PROFILE/DBT_TARGET(seedbt/cli/params.py) and selects that profile/target._initialize_dbt_core_adapter, used for metadata calls such as column metadata): calls dbt'sload_profile(self.project_dir, cli_vars, self.profile, self.target).self.profile/self.targetareNoneunless explicitly set on the resource, and dbt'sload_profiledoes not read the env vars itself — only its Click CLI layer does.As a result, with
DBT_TARGET=prodset in the environment, the subprocess usesprodwhile the in-process adapter falls back to theprofiles.ymldefault (e.g.dev), so metadata calls connect to the wrong warehouse/credentials. This is the exact symptom reported in #33818.The fix mirrors dbt's CLI resolution: fall back to
DBT_PROFILE/DBT_TARGETwhen the resource does not set profile/target explicitly. Explicit resource configuration still takes precedence, matching dbt's documented--profile/--targetoverride semantics.(
DBT_PROFILES_DIRis a separate, lower-priority gap mentioned in the issue title; the reproduced symptom isDBT_TARGET/DBT_PROFILE, so this PR is kept minimal to those.)How I Tested These Changes
Added a parametrized unit test in
dagster_dbt_tests/core/test_resource.pythat patches dbt'sload_profileand asserts the exactprofile/targetoverrides Dagster passes to it, then short-circuits before any real adapter/warehouse connection:DBT_PROFILE/DBT_TARGETare honored when the resource sets neither;profile=/target=on the resource win over the env vars;Noneis passed through so dbt applies its own default resolution.ruff checkandruff format --checkpass on both changed files.Changelog
[dagster-dbt]DbtCliResourcenow honors theDBT_PROFILEandDBT_TARGETenvironment variables in its in-process adapter (used for metadata), matching how the dbt subprocess already resolves them. Explicitprofile/targetconfiguration still takes precedence.Prepared with AI assistance (Claude Code).