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

Allow setting access in config in addition to properties #8635

Merged
merged 4 commits into from
Sep 14, 2023
Merged
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/Features-20230912-153013.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Allow setting "access" as a config in addition to as a property
time: 2023-09-12T15:30:13.859595-04:00
custom:
Author: gshank
Issue: "8383"
10 changes: 9 additions & 1 deletion core/dbt/contracts/graph/model_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from dbt.contracts.util import Replaceable, list_str
from dbt.exceptions import DbtInternalError, CompilationError
from dbt import hooks
from dbt.node_types import NodeType
from dbt.node_types import NodeType, AccessType
from mashumaro.jsonschema.annotations import Pattern


Expand Down Expand Up @@ -518,6 +518,14 @@ def field_mapping(cls):
return {"post_hook": "post-hook", "pre_hook": "pre-hook"}


@dataclass
class ModelConfig(NodeConfig):
access: AccessType = field(
default=AccessType.Protected,
metadata=MergeBehavior.Update.meta(),
)


@dataclass
class SeedConfig(NodeConfig):
materialized: str = "seed"
Expand Down
4 changes: 3 additions & 1 deletion core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@

from .model_config import (
NodeConfig,
ModelConfig,
SeedConfig,
TestConfig,
SourceConfig,
Expand Down Expand Up @@ -569,6 +570,7 @@ class HookNode(CompiledNode):
class ModelNode(CompiledNode):
resource_type: Literal[NodeType.Model]
access: AccessType = AccessType.Protected
config: ModelConfig = field(default_factory=ModelConfig)
constraints: List[ModelLevelConstraint] = field(default_factory=list)
version: Optional[NodeVersion] = None
latest_version: Optional[NodeVersion] = None
Expand Down Expand Up @@ -605,7 +607,7 @@ def from_args(cls, args: ModelNodeArgs) -> "ModelNode":
path="",
unrendered_config=unrendered_config,
depends_on=DependsOn(nodes=args.depends_on_nodes),
config=NodeConfig(enabled=args.enabled),
config=ModelConfig(enabled=args.enabled),
)

@property
Expand Down
18 changes: 16 additions & 2 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
from dbt.contracts.graph.manifest import Manifest
from dbt.contracts.graph.nodes import Contract, BaseNode, ManifestNode
from dbt.contracts.graph.unparsed import Docs, UnparsedNode
from dbt.exceptions import DbtInternalError, ConfigUpdateError, DictParseError
from dbt.exceptions import (
DbtInternalError,
ConfigUpdateError,
DictParseError,
InvalidAccessTypeError,
)
from dbt import hooks
from dbt.node_types import NodeType, ModelLanguage
from dbt.node_types import NodeType, ModelLanguage, AccessType
from dbt.parser.search import FileBlock

# internally, the parser may store a less-restrictive type that will be
Expand Down Expand Up @@ -328,6 +333,15 @@
if "group" in config_dict and config_dict["group"]:
parsed_node.group = config_dict["group"]

# If we have access in the config, copy to node level
if parsed_node.resource_type == NodeType.Model and config_dict.get("access", None):
if AccessType.is_valid(config_dict["access"]):
Copy link
Member

Choose a reason for hiding this comment

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

Do we need nested if statements here, or can we combine into 1 level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They can't be combined, the logic would be wrong.

parsed_node.access = AccessType(config_dict["access"])

Check warning on line 339 in core/dbt/parser/base.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/base.py#L338-L339

Added lines #L338 - L339 were not covered by tests
else:
raise InvalidAccessTypeError(

Check warning on line 341 in core/dbt/parser/base.py

View check run for this annotation

Codecov / codecov/patch

core/dbt/parser/base.py#L341

Added line #L341 was not covered by tests
unique_id=parsed_node.unique_id, field_value=config_dict["access"]
)

# If we have docs in the config, merge with the node level, for backwards
# compatibility with earlier node-only config.
if "docs" in config_dict and config_dict["docs"]:
Expand Down
4 changes: 4 additions & 0 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ def get_unparsed_target(self) -> Iterable[NonSourceTarget]:
self.normalize_docs_attribute(data, path)
self.normalize_group_attribute(data, path)
self.normalize_contract_attribute(data, path)
self.normalize_access_attribute(data, path)
node = self._target_type().from_dict(data)
except (ValidationError, JSONValidationError) as exc:
raise YamlParseDictError(path, self.key, data, exc)
Expand Down Expand Up @@ -503,6 +504,9 @@ def normalize_group_attribute(self, data, path):
def normalize_contract_attribute(self, data, path):
return self.normalize_attribute(data, path, "contract")

def normalize_access_attribute(self, data, path):
return self.normalize_attribute(data, path, "access")

def patch_node_config(self, node, patch):
# Get the ContextConfig that's used in calculating the config
# This must match the model resource_type that's being patched
Expand Down
201 changes: 199 additions & 2 deletions schemas/dbt/manifest/v11.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
},
"dbt_version": {
"type": "string",
"default": "1.7.0b1"
"default": "1.7.0b2"
},
"generated_at": {
"type": "string"
Expand Down Expand Up @@ -1463,6 +1463,203 @@
"checksum"
]
},
"ModelConfig": {
"type": "object",
"title": "ModelConfig",
"properties": {
"_extra": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"enabled": {
"type": "boolean",
"default": true
},
"alias": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null
},
"schema": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null
},
"database": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null
},
"tags": {
"anyOf": [
{
"type": "array",
"items": {
"type": "string"
}
},
{
"type": "string"
}
]
},
"meta": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"group": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null
},
"materialized": {
"type": "string",
"default": "view"
},
"incremental_strategy": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": null
},
"persist_docs": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"post-hook": {
"type": "array",
"items": {
"$ref": "#/$defs/Hook"
}
},
"pre-hook": {
"type": "array",
"items": {
"$ref": "#/$defs/Hook"
}
},
"quoting": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"column_types": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"full_refresh": {
"anyOf": [
{
"type": "boolean"
},
{
"type": "null"
}
],
"default": null
},
"unique_key": {
"anyOf": [
{
"type": "string"
},
{
"type": "array",
"items": {
"type": "string"
}
},
{
"type": "null"
}
],
"default": null
},
"on_schema_change": {
"anyOf": [
{
"type": "string"
},
{
"type": "null"
}
],
"default": "ignore"
},
"on_configuration_change": {
"enum": [
"apply",
"continue",
"fail"
]
},
"grants": {
"type": "object",
"propertyNames": {
"type": "string"
}
},
"packages": {
"type": "array",
"items": {
"type": "string"
}
},
"docs": {
"$ref": "#/$defs/Docs"
},
"contract": {
"$ref": "#/$defs/ContractConfig"
},
"access": {
"enum": [
"private",
"protected",
"public"
],
"default": "protected"
}
},
"additionalProperties": true
},
"ModelLevelConstraint": {
"type": "object",
"title": "ModelLevelConstraint",
Expand Down Expand Up @@ -1606,7 +1803,7 @@
"$ref": "#/$defs/FileHash"
},
"config": {
"$ref": "#/$defs/NodeConfig"
"$ref": "#/$defs/ModelConfig"
},
"_event_status": {
"type": "object",
Expand Down
Loading
Loading