-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[dagster-dbt] Honor DBT_PROFILE/DBT_TARGET in the in-process adapter #33937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -386,7 +386,16 @@ def _initialize_dbt_core_adapter(self, args: Sequence[str]) -> BaseAdapter: | |||||||||
| set_from_args(Namespace(profiles_dir=profiles_dir), None) | ||||||||||
| flags = get_flags() | ||||||||||
|
|
||||||||||
| profile = load_profile(self.project_dir, cli_vars, self.profile, self.target) | ||||||||||
| # Mirror dbt's CLI flag/env-var resolution. dbt only reads DBT_PROFILE / | ||||||||||
| # DBT_TARGET at its Click CLI layer (dbt/cli/params.py), so the `dbt` | ||||||||||
| # subprocess this resource spawns honors them, but the in-process adapter | ||||||||||
| # used for metadata calls (e.g. column metadata) does not. Fall back to the | ||||||||||
| # 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") | ||||||||||
| target_override = self.target or os.getenv("DBT_TARGET") | ||||||||||
|
Comment on lines
+396
to
+397
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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!
Comment on lines
+396
to
+397
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In dbt-core 1.7.x, which Useful? React with 👍 / 👎.
Comment on lines
+396
to
+397
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a caller supplies Useful? React with 👍 / 👎. |
||||||||||
| profile = load_profile(self.project_dir, cli_vars, profile_override, target_override) | ||||||||||
| project = load_project(self.project_dir, False, profile, cli_vars) | ||||||||||
| config = RuntimeConfig.from_parts(project, profile, flags) | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,6 +264,67 @@ def test_dbt_profile_configuration() -> None: | |
| assert dbt_cli_invocation.is_successful() | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("explicit_profile", "explicit_target", "env", "expected_profile", "expected_target"), | ||
| [ | ||
| # DBT_PROFILE / DBT_TARGET are honored when the resource sets neither, so the | ||
| # in-process adapter resolves the same profile as the dbt subprocess. | ||
| (None, None, {"DBT_PROFILE": "env_profile", "DBT_TARGET": "prod"}, "env_profile", "prod"), | ||
| # Explicit resource config always wins over the env vars. | ||
| ( | ||
| "arg_profile", | ||
| "arg_target", | ||
| {"DBT_PROFILE": "env_profile", "DBT_TARGET": "prod"}, | ||
| "arg_profile", | ||
| "arg_target", | ||
| ), | ||
| # Nothing set: pass None through so dbt applies its own profiles.yml default. | ||
| (None, None, {}, None, None), | ||
| ], | ||
| ids=["env_var_honored", "explicit_arg_wins", "no_override"], | ||
| ) | ||
| def test_initialize_dbt_core_adapter_honors_target_profile_env_vars( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| mocker: MockerFixture, | ||
| explicit_profile: str | None, | ||
| explicit_target: str | None, | ||
| env: dict[str, str], | ||
| expected_profile: str | None, | ||
| expected_target: str | None, | ||
| ) -> None: | ||
| # https://github.com/dagster-io/dagster/issues/33818 - the in-process adapter | ||
| # used for metadata calls ignored DBT_PROFILE / DBT_TARGET while the dbt | ||
| # subprocess honored them (via dbt's own CLI layer), so the two paths could | ||
| # resolve different profiles/targets. | ||
| 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) | ||
|
|
||
|
Comment on lines
+299
to
+306
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Several real dbt calls run inside |
||
| # Capture the overrides Dagster passes to dbt, then stop before any real | ||
| # adapter / warehouse connection is created. | ||
| mock_load_profile = mocker.patch( | ||
| "dbt.config.runtime.load_profile", side_effect=_StopAdapterInit | ||
| ) | ||
|
|
||
| resource = DbtCliResource( | ||
| project_dir=os.fspath(test_jaffle_shop_path), | ||
| profile=explicit_profile, | ||
| target=explicit_target, | ||
| ) | ||
|
|
||
| with pytest.raises(_StopAdapterInit): | ||
| resource._initialize_dbt_core_adapter(["run"]) # noqa: SLF001 | ||
|
|
||
| # load_profile(project_dir, cli_vars, profile_name_override, target_override) | ||
| _, _, passed_profile, passed_target = mock_load_profile.call_args.args | ||
| assert passed_profile == expected_profile | ||
| assert passed_target == expected_target | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "profiles_dir", [None, test_jaffle_shop_path, os.fspath(test_jaffle_shop_path)] | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
DBT_PROFILEis set together withDBT_PROFILES_DIRand the resource has no explicitprofiles_dir, the subprocess inheritsDBT_PROFILES_DIR, but_initialize_dbt_core_adapterstill sets dbt flags toself.project_dirbefore callingload_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 👍 / 👎.