Skip to content

Commit

Permalink
Revert "state:modified vars, behind "state_modified_compare_vars" beh…
Browse files Browse the repository at this point in the history
…aviour flag" (#10793)
  • Loading branch information
aranke committed Oct 2, 2024
1 parent 7940ad5 commit 9844540
Show file tree
Hide file tree
Showing 22 changed files with 22 additions and 625 deletions.
7 changes: 0 additions & 7 deletions .changes/1.9.0/Features-20240729-173203.yaml

This file was deleted.

1 change: 0 additions & 1 deletion core/dbt/artifacts/resources/v1/components.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ class ParsedResource(ParsedResourceMandatory):
unrendered_config_call_dict: Dict[str, Any] = field(default_factory=dict)
relation_name: Optional[str] = None
raw_code: str = ""
vars: Dict[str, Any] = field(default_factory=dict)

def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None):
dct = super().__post_serialize__(dct, context)
Expand Down
1 change: 0 additions & 1 deletion core/dbt/artifacts/resources/v1/exposure.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class Exposure(GraphResource):
tags: List[str] = field(default_factory=list)
config: ExposureConfig = field(default_factory=ExposureConfig)
unrendered_config: Dict[str, Any] = field(default_factory=dict)
vars: Dict[str, Any] = field(default_factory=dict)
url: Optional[str] = None
depends_on: DependsOn = field(default_factory=DependsOn)
refs: List[RefArgs] = field(default_factory=list)
Expand Down
1 change: 0 additions & 1 deletion core/dbt/artifacts/resources/v1/source_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class SourceDefinition(ParsedSourceMandatory):
config: SourceConfig = field(default_factory=SourceConfig)
patch_path: Optional[str] = None
unrendered_config: Dict[str, Any] = field(default_factory=dict)
vars: Dict[str, Any] = field(default_factory=dict)
relation_name: Optional[str] = None
created_at: float = field(default_factory=lambda: time.time())
unrendered_database: Optional[str] = None
Expand Down
38 changes: 12 additions & 26 deletions core/dbt/context/configured.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,35 +31,23 @@ def __init__(self, package_name: str):
self.resource_type = NodeType.Model


class SchemaYamlVars:
def __init__(self):
self.env_vars = {}
self.vars = {}


class ConfiguredVar(Var):
def __init__(
self,
context: Dict[str, Any],
config: AdapterRequiredConfig,
project_name: str,
schema_yaml_vars: Optional[SchemaYamlVars] = None,
):
super().__init__(context, config.cli_vars)
self._config = config
self._project_name = project_name
self.schema_yaml_vars = schema_yaml_vars

def __call__(self, var_name, default=Var._VAR_NOTSET):
my_config = self._config.load_dependencies()[self._project_name]

var_found = False
var_value = None

# cli vars > active project > local project
if var_name in self._config.cli_vars:
var_found = True
var_value = self._config.cli_vars[var_name]
return self._config.cli_vars[var_name]

adapter_type = self._config.credentials.type
lookup = FQNLookup(self._project_name)
Expand All @@ -70,21 +58,19 @@ def __call__(self, var_name, default=Var._VAR_NOTSET):
all_vars.add(my_config.vars.vars_for(lookup, adapter_type))
all_vars.add(active_vars)

if not var_found and var_name in all_vars:
var_found = True
var_value = all_vars[var_name]
if var_name in all_vars:
return all_vars[var_name]

if not var_found and default is not Var._VAR_NOTSET:
var_found = True
var_value = default
if default is not Var._VAR_NOTSET:
return default

if not var_found:
return self.get_missing_var(var_name)
else:
if self.schema_yaml_vars:
self.schema_yaml_vars.vars[var_name] = var_value
return self.get_missing_var(var_name)

Check warning on line 67 in core/dbt/context/configured.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/context/configured.py#L67

Added line #L67 was not covered by tests

return var_value

class SchemaYamlVars:
def __init__(self):
self.env_vars = {}
self.vars = {}


class SchemaYamlContext(ConfiguredContext):
Expand All @@ -96,7 +82,7 @@ def __init__(self, config, project_name: str, schema_yaml_vars: Optional[SchemaY

@contextproperty()
def var(self) -> ConfiguredVar:
return ConfiguredVar(self._ctx, self.config, self._project_name, self.schema_yaml_vars)
return ConfiguredVar(self._ctx, self.config, self._project_name)

@contextmember()
def env_var(self, var: str, default: Optional[str] = None) -> str:
Expand Down
8 changes: 0 additions & 8 deletions core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -790,14 +790,6 @@ def get_missing_var(self, var_name):
# in the parser, just always return None.
return None

def __call__(self, var_name: str, default: Any = ModelConfiguredVar._VAR_NOTSET) -> Any:
var_value = super().__call__(var_name, default)

if self._node and hasattr(self._node, "vars"):
self._node.vars[var_name] = var_value

return var_value


class RuntimeVar(ModelConfiguredVar):
pass
Expand Down
17 changes: 0 additions & 17 deletions core/dbt/contracts/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ class SchemaSourceFile(BaseSourceFile):
unrendered_configs: Dict[str, Any] = field(default_factory=dict)
unrendered_databases: Dict[str, Any] = field(default_factory=dict)
unrendered_schemas: Dict[str, Any] = field(default_factory=dict)
vars: Dict[str, Any] = field(default_factory=dict)
pp_dict: Optional[Dict[str, Any]] = None
pp_test_index: Optional[Dict[str, Any]] = None

Expand Down Expand Up @@ -356,22 +355,6 @@ def delete_from_unrendered_configs(self, yaml_key, name):
if not self.unrendered_configs[yaml_key]:
del self.unrendered_configs[yaml_key]

def add_vars(self, vars: Dict[str, Any], yaml_key: str, name: str) -> None:
if yaml_key not in self.vars:
self.vars[yaml_key] = {}

if name not in self.vars[yaml_key]:
self.vars[yaml_key][name] = vars

def get_vars(self, yaml_key: str, name: str) -> Dict[str, Any]:
if yaml_key not in self.vars:
return {}

if name not in self.vars[yaml_key]:
return {}

return self.vars[yaml_key][name]

def add_env_var(self, var, yaml_key, name):
if yaml_key not in self.env_vars:
self.env_vars[yaml_key] = {}
Expand Down
32 changes: 0 additions & 32 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,30 +369,19 @@ def same_contract(self, old, adapter_type=None) -> bool:
# This would only apply to seeds
return True

def same_vars(self, old) -> bool:
return self.vars == old.vars

def same_contents(self, old, adapter_type) -> bool:
if old is None:
return False

# Need to ensure that same_contract is called because it
# could throw an error
same_contract = self.same_contract(old, adapter_type)

# Legacy behaviour
if not get_flags().state_modified_compare_vars:
same_vars = True
else:
same_vars = self.same_vars(old)

return (
self.same_body(old)
and self.same_config(old)
and self.same_persisted_description(old)
and self.same_fqn(old)
and self.same_database_representation(old)
and same_vars
and same_contract
and True
)
Expand Down Expand Up @@ -1266,9 +1255,6 @@ def same_config(self, old: "SourceDefinition") -> bool:
old.unrendered_config,
)

def same_vars(self, other: "SourceDefinition") -> bool:
return self.vars == other.vars

def same_contents(self, old: Optional["SourceDefinition"]) -> bool:
# existing when it didn't before is a change!
if old is None:
Expand All @@ -1282,20 +1268,13 @@ def same_contents(self, old: Optional["SourceDefinition"]) -> bool:
# freshness changes are changes, I guess
# metadata/tags changes are not "changes"
# patching/description changes are not "changes"
# Legacy behaviour
if not get_flags().state_modified_compare_vars:
same_vars = True
else:
same_vars = self.same_vars(old)

return (
self.same_database_representation(old)
and self.same_fqn(old)
and self.same_config(old)
and self.same_quoting(old)
and self.same_freshness(old)
and self.same_external(old)
and same_vars
and True
)

Expand Down Expand Up @@ -1392,21 +1371,12 @@ def same_config(self, old: "Exposure") -> bool:
old.unrendered_config,
)

def same_vars(self, old: "Exposure") -> bool:
return self.vars == old.vars

def same_contents(self, old: Optional["Exposure"]) -> bool:
# existing when it didn't before is a change!
# metadata/tags changes are not "changes"
if old is None:
return True

# Legacy behaviour
if not get_flags().state_modified_compare_vars:
same_vars = True
else:
same_vars = self.same_vars(old)

return (
self.same_fqn(old)
and self.same_exposure_type(old)
Expand All @@ -1417,7 +1387,6 @@ def same_contents(self, old: Optional["Exposure"]) -> bool:
and self.same_label(old)
and self.same_depends_on(old)
and self.same_config(old)
and same_vars
and True
)

Expand Down Expand Up @@ -1669,7 +1638,6 @@ class ParsedNodePatch(ParsedPatch):
latest_version: Optional[NodeVersion]
constraints: List[Dict[str, Any]]
deprecation_date: Optional[datetime]
vars: Dict[str, Any]
time_spine: Optional[TimeSpine] = None


Expand Down
2 changes: 0 additions & 2 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ class ProjectFlags(ExtensibleDbtClassMixin):
source_freshness_run_project_hooks: bool = False
skip_nodes_if_on_run_start_fails: bool = False
state_modified_compare_more_unrendered_values: bool = False
state_modified_compare_vars: bool = False

@property
def project_only_flags(self) -> Dict[str, Any]:
Expand All @@ -353,7 +352,6 @@ def project_only_flags(self) -> Dict[str, Any]:
"source_freshness_run_project_hooks": self.source_freshness_run_project_hooks,
"skip_nodes_if_on_run_start_fails": self.skip_nodes_if_on_run_start_fails,
"state_modified_compare_more_unrendered_values": self.state_modified_compare_more_unrendered_values,
"state_modified_compare_vars": self.state_modified_compare_vars,
}


Expand Down
1 change: 0 additions & 1 deletion core/dbt/graph/selector_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,6 @@ def search(self, included_nodes: Set[UniqueId], selector: str) -> Iterator[Uniqu
"modified.relation": self.check_modified_factory("same_database_representation"),
"modified.macros": self.check_modified_macros,
"modified.contract": self.check_modified_contract("same_contract", adapter_type),
"modified.vars": self.check_modified_factory("same_vars"),
}
if selector in state_checks:
checker = state_checks[selector]
Expand Down
5 changes: 1 addition & 4 deletions core/dbt/parser/schema_yaml_readers.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None:
unique_id = f"{NodeType.Exposure}.{package_name}.{unparsed.name}"
path = self.yaml.path.relative_path

assert isinstance(self.yaml.file, SchemaSourceFile)
exposure_vars = self.yaml.file.get_vars(self.key, unparsed.name)

fqn = self.schema_parser.get_fqn_prefix(path)
fqn.append(unparsed.name)

Expand Down Expand Up @@ -136,7 +133,6 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None:
maturity=unparsed.maturity,
config=config,
unrendered_config=unrendered_config,
vars=exposure_vars,
)
ctx = generate_parse_exposure(
parsed,
Expand All @@ -148,6 +144,7 @@ def parse_exposure(self, unparsed: UnparsedExposure) -> None:
get_rendered(depends_on_jinja, ctx, parsed, capture_macros=True)
# parsed now has a populated refs/sources/metrics

assert isinstance(self.yaml.file, SchemaSourceFile)
if parsed.config.enabled:
self.manifest.add_exposure(self.yaml.file, parsed)
else:
Expand Down
18 changes: 4 additions & 14 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,14 +431,10 @@ def get_key_dicts(self) -> Iterable[Dict[str, Any]]:

if self.schema_yaml_vars.env_vars:
self.schema_parser.manifest.env_vars.update(self.schema_yaml_vars.env_vars)
for env_var in self.schema_yaml_vars.env_vars.keys():
schema_file.add_env_var(env_var, self.key, entry["name"])
for var in self.schema_yaml_vars.env_vars.keys():
schema_file.add_env_var(var, self.key, entry["name"])
self.schema_yaml_vars.env_vars = {}

if self.schema_yaml_vars.vars:
schema_file.add_vars(self.schema_yaml_vars.vars, self.key, entry["name"])
self.schema_yaml_vars.vars = {}

yield entry

def render_entry(self, dct):
Expand Down Expand Up @@ -718,9 +714,6 @@ def parse_patch(self, block: TargetBlock[NodeTarget], refs: ParserRef) -> None:
# code consistency.
deprecation_date: Optional[datetime.datetime] = None
time_spine: Optional[TimeSpine] = None
assert isinstance(self.yaml.file, SchemaSourceFile)
source_file: SchemaSourceFile = self.yaml.file

if isinstance(block.target, UnparsedModelUpdate):
deprecation_date = block.target.deprecation_date
time_spine = (
Expand Down Expand Up @@ -753,9 +746,9 @@ def parse_patch(self, block: TargetBlock[NodeTarget], refs: ParserRef) -> None:
constraints=block.target.constraints,
deprecation_date=deprecation_date,
time_spine=time_spine,
vars=source_file.get_vars(block.target.yaml_key, block.target.name),
)

assert isinstance(self.yaml.file, SchemaSourceFile)
source_file: SchemaSourceFile = self.yaml.file
if patch.yaml_key in ["models", "seeds", "snapshots"]:
unique_id = self.manifest.ref_lookup.get_unique_id(
patch.name, self.project.project_name, None
Expand Down Expand Up @@ -849,8 +842,6 @@ def patch_node_properties(self, node, patch: "ParsedNodePatch") -> None:
node.description = patch.description
node.columns = patch.columns
node.name = patch.name
# Prefer node-level vars to vars from patch
node.vars = {**patch.vars, **node.vars}

if not isinstance(node, ModelNode):
for attr in ["latest_version", "access", "version", "constraints"]:
Expand Down Expand Up @@ -1000,7 +991,6 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef)
latest_version=latest_version,
constraints=unparsed_version.constraints or target.constraints,
deprecation_date=unparsed_version.deprecation_date,
vars=source_file.get_vars(block.target.yaml_key, block.target.name),
)
# Node patched before config because config patching depends on model name,
# which may have been updated in the version patch
Expand Down
6 changes: 0 additions & 6 deletions core/dbt/parser/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
ContextConfigGenerator,
UnrenderedConfigGenerator,
)
from dbt.contracts.files import SchemaSourceFile
from dbt.contracts.graph.manifest import Manifest, SourceKey
from dbt.contracts.graph.nodes import (
GenericTestNode,
Expand Down Expand Up @@ -159,10 +158,6 @@ def parse_source(self, target: UnpatchedSourceDefinition) -> SourceDefinition:
rendered=False,
)

schema_file = self.manifest.files[target.file_id]
assert isinstance(schema_file, SchemaSourceFile)
source_vars = schema_file.get_vars("sources", source.name)

if not isinstance(config, SourceConfig):
raise DbtInternalError(
f"Calculated a {type(config)} for a source, but expected a SourceConfig"
Expand Down Expand Up @@ -197,7 +192,6 @@ def parse_source(self, target: UnpatchedSourceDefinition) -> SourceDefinition:
tags=tags,
config=config,
unrendered_config=unrendered_config,
vars=source_vars,
)

if (
Expand Down
Loading

0 comments on commit 9844540

Please sign in to comment.