Skip to content

Commit

Permalink
Ct-581 grants as configs (#5230)
Browse files Browse the repository at this point in the history
* Handle 'grants' in NodeConfig, with correct merge behavior

* Fix a bunch of tests

* Add changie

* Actually add the test

* Change to default replace of grants with '+' extending them

* Additional tests, fix config_call_dict handling

* Tweak _add_config_call to remove unnecessary isinstance checks
  • Loading branch information
gshank authored May 19, 2022
1 parent ae62f57 commit e50678c
Show file tree
Hide file tree
Showing 16 changed files with 320 additions and 19 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Features-20220510-204949.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Features
body: Grants as Node Configs
time: 2022-05-10T20:49:49.197999-04:00
custom:
Author: gshank
Issue: "5189"
PR: "5230"
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ endif
dev: ## Installs dbt-* packages in develop mode along with development dependencies.
@\
pip install -r dev-requirements.txt -r editable-requirements.txt && \
pre-commit install

.PHONY: mypy
mypy: .env ## Runs mypy against staged changes for static type checking.
Expand Down
45 changes: 38 additions & 7 deletions core/dbt/context/context_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import List, Iterator, Dict, Any, TypeVar, Generic

from dbt.config import RuntimeConfig, Project, IsFQNResource
from dbt.contracts.graph.model_config import BaseConfig, get_config_for
from dbt.contracts.graph.model_config import BaseConfig, get_config_for, _listify
from dbt.exceptions import InternalException
from dbt.node_types import NodeType
from dbt.utils import fqn_search
Expand Down Expand Up @@ -264,18 +264,49 @@ def add_config_call(self, opts: Dict[str, Any]) -> None:

@classmethod
def _add_config_call(cls, config_call_dict, opts: Dict[str, Any]) -> None:
# config_call_dict is already encountered configs, opts is new
# This mirrors code in _merge_field_value in model_config.py which is similar but
# operates on config objects.
for k, v in opts.items():
# MergeBehavior for post-hook and pre-hook is to collect all
# values, instead of overwriting
if k in BaseConfig.mergebehavior["append"]:
if not isinstance(v, list):
v = [v]
if k in BaseConfig.mergebehavior["update"] and not isinstance(v, dict):
raise InternalException(f"expected dict, got {v}")
if k in config_call_dict and isinstance(config_call_dict[k], list):
config_call_dict[k].extend(v)
elif k in config_call_dict and isinstance(config_call_dict[k], dict):
config_call_dict[k].update(v)
if k in config_call_dict: # should always be a list here
config_call_dict[k].extend(v)
else:
config_call_dict[k] = v

elif k in BaseConfig.mergebehavior["update"]:
if not isinstance(v, dict):
raise InternalException(f"expected dict, got {v}")
if k in config_call_dict and isinstance(config_call_dict[k], dict):
config_call_dict[k].update(v)
else:
config_call_dict[k] = v
elif k in BaseConfig.mergebehavior["dict_key_append"]:
if not isinstance(v, dict):
raise InternalException(f"expected dict, got {v}")
if k in config_call_dict: # should always be a dict
for key, value in v.items():
extend = False
# This might start with a +, to indicate we should extend the list
# instead of just clobbering it
if key.startswith("+"):
extend = True
if key in config_call_dict[k] and extend:
# extend the list
config_call_dict[k][key].extend(_listify(value))
else:
# clobber the list
config_call_dict[k][key] = _listify(value)
else:
# This is always a dictionary
config_call_dict[k] = v
# listify everything
for key, value in config_call_dict[k].items():
config_call_dict[k][key] = _listify(value)
else:
config_call_dict[k] = v

Expand Down
6 changes: 6 additions & 0 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,12 @@ class WritableManifest(ArtifactMixin):
)
)

def __post_serialize__(self, dct):
for unique_id, node in dct["nodes"].items():
if "config_call_dict" in node:
del node["config_call_dict"]
return dct


def _check_duplicates(value: HasUniqueID, src: Mapping[str, HasUniqueID]):
if value.unique_id in src:
Expand Down
33 changes: 33 additions & 0 deletions core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class MergeBehavior(Metadata):
Append = 1
Update = 2
Clobber = 3
DictKeyAppend = 4

@classmethod
def default_field(cls) -> "MergeBehavior":
Expand Down Expand Up @@ -124,6 +125,9 @@ def _listify(value: Any) -> List:
return [value]


# There are two versions of this code. The one here is for config
# objects, the one in _add_config_call in context_config.py is for
# config_call_dict dictionaries.
def _merge_field_value(
merge_behavior: MergeBehavior,
self_value: Any,
Expand All @@ -141,6 +145,31 @@ def _merge_field_value(
value = self_value.copy()
value.update(other_value)
return value
elif merge_behavior == MergeBehavior.DictKeyAppend:
if not isinstance(self_value, dict):
raise InternalException(f"expected dict, got {self_value}")
if not isinstance(other_value, dict):
raise InternalException(f"expected dict, got {other_value}")
new_dict = {}
for key in self_value.keys():
new_dict[key] = _listify(self_value[key])
for key in other_value.keys():
extend = False
new_key = key
# This might start with a +, to indicate we should extend the list
# instead of just clobbering it
if new_key.startswith("+"):
new_key = key.lstrip("+")
extend = True
if new_key in new_dict and extend:
# extend the list
value = other_value[key]
new_dict[new_key].extend(_listify(value))
else:
# clobber the list
new_dict[new_key] = _listify(other_value[key])
return new_dict

else:
raise InternalException(f"Got an invalid merge_behavior: {merge_behavior}")

Expand Down Expand Up @@ -257,6 +286,7 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo
mergebehavior = {
"append": ["pre-hook", "pre_hook", "post-hook", "post_hook", "tags"],
"update": ["quoting", "column_types", "meta"],
"dict_key_append": ["grants"],
}

@classmethod
Expand Down Expand Up @@ -427,6 +457,9 @@ class NodeConfig(NodeAndTestConfig):
# sometimes getting the Union order wrong, causing serialization failures.
unique_key: Union[str, List[str], None] = None
on_schema_change: Optional[str] = "ignore"
grants: Dict[str, Any] = field(
default_factory=dict, metadata=MergeBehavior.DictKeyAppend.meta()
)

@classmethod
def __pre_deserialize__(cls, data):
Expand Down
2 changes: 0 additions & 2 deletions core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,6 @@ def _serialize(self):
return self.to_dict()

def __post_serialize__(self, dct):
if "config_call_dict" in dct:
del dct["config_call_dict"]
if "_event_status" in dct:
del dct["_event_status"]
return dct
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ def update_parsed_node_config(

# build_config_dict takes the config_call_dict in the ContextConfig object
# and calls calculate_node_config to combine dbt_project configs and
# config calls from SQL files
# config calls from SQL files, plus patch configs (from schema files)
config_dict = config.build_config_dict(patch_config_dict=patch_config_dict)

# Set tags on node provided in config blocks. Tags are additive, so even if
Expand Down
6 changes: 5 additions & 1 deletion core/dbt/tests/fixtures/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,11 @@ def dbt_project_yml(project_root, project_config_update, logs_dir):
"log-path": logs_dir,
}
if project_config_update:
project_config.update(project_config_update)
if isinstance(project_config_update, dict):
project_config.update(project_config_update)
elif isinstance(project_config_update, str):
updates = yaml.safe_load(project_config_update)
project_config.update(updates)
write_file(yaml.safe_dump(project_config), project_root, "dbt_project.yml")
return project_config

Expand Down
14 changes: 10 additions & 4 deletions test/unit/test_contracts_graph_compiled.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def basic_uncompiled_dict():
'tags': [],
'on_schema_change': 'ignore',
'meta': {},
'grants': {},
},
'docs': {'show': True},
'columns': {},
Expand All @@ -146,7 +147,8 @@ def basic_uncompiled_dict():
'extra_ctes': [],
'extra_ctes_injected': False,
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
'unrendered_config': {}
'unrendered_config': {},
'config_call_dict': {},
}


Expand Down Expand Up @@ -183,6 +185,7 @@ def basic_compiled_dict():
'tags': [],
'on_schema_change': 'ignore',
'meta': {},
'grants': {},
},
'docs': {'show': True},
'columns': {},
Expand All @@ -192,7 +195,8 @@ def basic_compiled_dict():
'extra_ctes_injected': True,
'compiled_sql': 'with whatever as (select * from other) select * from whatever',
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
'unrendered_config': {}
'unrendered_config': {},
'config_call_dict': {},
}


Expand Down Expand Up @@ -445,7 +449,8 @@ def basic_uncompiled_schema_test_dict():
'kwargs': {},
},
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
'unrendered_config': {}
'unrendered_config': {},
'config_call_dict': {},
}


Expand Down Expand Up @@ -497,7 +502,8 @@ def basic_compiled_schema_test_dict():
'checksum': {'name': 'sha256', 'checksum': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'},
'unrendered_config': {
'severity': 'warn',
}
},
'config_call_dict': {},
}


Expand Down
Loading

0 comments on commit e50678c

Please sign in to comment.