Skip to content
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

[Bug] Behavior flags should take the default value when the project file is not loaded #338

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20241024-154518.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Always make behavior flags available for evaluation
time: 2024-10-24T15:45:18.114936-04:00
custom:
Author: mikealfare
Issue: "338"
13 changes: 7 additions & 6 deletions dbt/adapters/base/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,13 @@ def behavior(self) -> Behavior:
@behavior.setter # type: ignore
def behavior(self, flags: List[BehaviorFlag]) -> None:
flags.extend(self._behavior_flags)
try:
# we don't always get project flags, for example during `dbt debug`
self._behavior = Behavior(flags, self.config.flags)
except AttributeError:
# in that case, don't load any behavior to avoid unexpected defaults
self._behavior = Behavior([], {})

# we don't always get project flags, for example, the project file is not loaded during `dbt debug`
# in that case, load the default values for behavior flags to avoid compilation errors
# this mimics not loading a project file, or not specifying flags in a project file
user_overrides = getattr(self.config, "flags", {})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is effectively the same as swapping [] for flags on line 308, but doing this reads much more cleanly.


self._behavior = Behavior(flags, user_overrides)

@property
def _behavior_flags(self) -> List[BehaviorFlag]:
Expand Down
Empty file.
87 changes: 87 additions & 0 deletions tests/unit/behavior_flag_tests/test_empty_project.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
from types import SimpleNamespace
from typing import Any, Dict, List

from dbt_common.behavior_flags import BehaviorFlag
from dbt_common.exceptions import DbtBaseException
import pytest

from dbt.adapters.contracts.connection import AdapterRequiredConfig, QueryComment

from tests.unit.fixtures.credentials import CredentialsStub


@pytest.fixture
def flags() -> Dict[str, Any]:
return {
"unregistered_flag": True,
"default_false_user_false_flag": False,
"default_false_user_true_flag": True,
"default_true_user_false_flag": False,
"default_true_user_true_flag": True,
}


@pytest.fixture
def config(flags) -> AdapterRequiredConfig:
raw_config = {
"credentials": CredentialsStub("test_database", "test_schema"),
"profile_name": "test_profile",
"target_name": "test_target",
"threads": 4,
"project_name": "test_project",
"query_comment": QueryComment(),
"cli_vars": {},
"target_path": "path/to/nowhere",
"log_cache_events": False,
}
return SimpleNamespace(**raw_config)


@pytest.fixture
def behavior_flags() -> List[BehaviorFlag]:
return [
{
"name": "default_false_user_false_flag",
"default": False,
"docs_url": "https://docs.com",
},
{
"name": "default_false_user_true_flag",
"default": False,
"description": "This is a false flag.",
},
{
"name": "default_false_user_skip_flag",
"default": False,
"description": "This is a true flag.",
},
{
"name": "default_true_user_false_flag",
"default": True,
"description": "This is fake news.",
},
{
"name": "default_true_user_true_flag",
"default": True,
"docs_url": "https://moar.docs.com",
},
{
"name": "default_true_user_skip_flag",
"default": True,
"description": "This is a true flag.",
},
]


def test_register_behavior_flags(adapter):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same test as in ./test_behavior_flags.py, but with flags removed from the config fixture above, and swapping the expected values for the two cases where the user override changes the result.

# make sure that users cannot add arbitrary flags to this collection
with pytest.raises(DbtBaseException):
assert adapter.behavior.unregistered_flag

# check the values of the valid behavior flags
assert not adapter.behavior.default_false_user_false_flag
assert not adapter.behavior.default_false_user_true_flag
assert not adapter.behavior.default_false_user_skip_flag
assert adapter.behavior.default_true_user_false_flag
assert adapter.behavior.default_true_user_true_flag
assert adapter.behavior.default_true_user_skip_flag
Loading