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

Model Deprecation #7562

Merged
merged 12 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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-20230509-233329.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Features
body: Added warnings for model and ref deprecations
time: 2023-05-09T23:33:29.679333-04:00
custom:
Author: peterallenwebb
Issue: "7433"
7 changes: 4 additions & 3 deletions core/dbt/config/renderer.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Dict, Any, Tuple, Optional, Union, Callable
import re
import os
from datetime import date

from dbt.clients.jinja import get_rendered, catch_jinja
from dbt.constants import SECRET_ENV_PREFIX
Expand Down Expand Up @@ -33,10 +34,10 @@ def render_entry(self, value: Any, keypath: Keypath) -> Any:
return self.render_value(value, keypath)

def render_value(self, value: Any, keypath: Optional[Keypath] = None) -> Any:
# keypath is ignored.
# if it wasn't read as a string, ignore it
# keypath is ignored (and someone who knows should explain why here)
if not isinstance(value, str):
return value
return value if not isinstance(value, date) else value.isoformat()

try:
with catch_jinja():
return get_rendered(value, self.context, native=True)
Expand Down
18 changes: 18 additions & 0 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,24 @@ def analysis_lookup(self) -> AnalysisLookup:
self._analysis_lookup = AnalysisLookup(self)
return self._analysis_lookup

def resolve_refs(
self, source_node: GraphMemberNode, current_project: str
) -> List[MaybeNonSource]:
resolved_refs: List[MaybeNonSource] = []
for ref in source_node.refs:
resolved = self.resolve_ref(
source_node,
ref.name,
ref.package,
ref.version,
current_project,
source_node.package_name,
)
resolved_refs.append(resolved)

return resolved_refs

# TODO: Simplify this function by passing the ref (target) instead of the ref's properties
# Called by dbt.parser.manifest._process_refs_for_exposure, _process_refs_for_metric,
# and dbt.parser.manifest._process_refs_for_node
def resolve_ref(
Expand Down
4 changes: 4 additions & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
from datetime import datetime
import time
from dataclasses import dataclass, field
from enum import Enum
Expand Down Expand Up @@ -478,6 +479,7 @@ def patch(self, patch: "ParsedNodePatch"):
)
self.version = patch.version
self.latest_version = patch.latest_version
self.deprecation_date = patch.deprecation_date

# This might not be the ideal place to validate the "access" field,
# but at this point we have the information we need to properly
Expand Down Expand Up @@ -685,6 +687,7 @@ class ModelNode(CompiledNode):
constraints: List[ModelLevelConstraint] = field(default_factory=list)
version: Optional[NodeVersion] = None
latest_version: Optional[NodeVersion] = None
deprecation_date: Optional[datetime] = None

@property
def is_latest_version(self) -> bool:
Expand Down Expand Up @@ -1391,6 +1394,7 @@ class ParsedNodePatch(ParsedPatch):
access: Optional[str]
version: Optional[NodeVersion]
latest_version: Optional[NodeVersion]
deprecation_date: Optional[datetime]


@dataclass
Expand Down
22 changes: 22 additions & 0 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import datetime
import re

from dbt import deprecations
Expand Down Expand Up @@ -154,6 +155,7 @@ class UnparsedVersion(dbtClassMixin):
columns: Sequence[Union[dbt.helper_types.IncludeExclude, UnparsedColumn]] = field(
default_factory=list
)
deprecation_date: Optional[datetime.datetime] = None

def __lt__(self, other):
try:
Expand Down Expand Up @@ -192,6 +194,8 @@ def __post_init__(self):
else:
self._unparsed_columns.append(column)

self.deprecation_date = normalize_date(self.deprecation_date)


@dataclass
class UnparsedAnalysisUpdate(HasConfig, HasColumnDocs, HasColumnProps, HasYamlMetadata):
Expand All @@ -210,6 +214,7 @@ class UnparsedModelUpdate(UnparsedNodeUpdate):
access: Optional[str] = None
latest_version: Optional[NodeVersion] = None
versions: Sequence[UnparsedVersion] = field(default_factory=list)
deprecation_date: Optional[datetime.datetime] = None

def __post_init__(self):
if self.latest_version:
Expand All @@ -229,6 +234,8 @@ def __post_init__(self):

self._version_map = {version.v: version for version in self.versions}

self.deprecation_date = normalize_date(self.deprecation_date)

def get_columns_for_version(self, version: NodeVersion) -> List[UnparsedColumn]:
if version not in self._version_map:
raise DbtInternalError(
Expand Down Expand Up @@ -652,3 +659,18 @@ def validate(cls, data):
super(UnparsedGroup, cls).validate(data)
if data["owner"].get("name") is None and data["owner"].get("email") is None:
raise ValidationError("Group owner must have at least one of 'name' or 'email'.")


def normalize_date(d: Optional[datetime.date]) -> Optional[datetime.datetime]:
"""Convert date to datetime (at midnight), and add local time zone if naive"""
if d is None:
return None

# convert date to datetime
dt = d if type(d) == datetime.datetime else datetime.datetime(d.year, d.month, d.day)

if not dt.tzinfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

The Python docs here say the following:

image

Translating those into code gives something like this.

The slight change suggested below would cover the case where tzinfo is not None but dt.tzinfo.utcoffset(dt) returns None.

To fully encode the Python datetime definition, I think we'd need to do something like this:

Suggested change
if not dt.tzinfo:
if not dt.tzinfo or not dt.tzinfo.utcoffset(dt):

Granted, it might "never" happen for a tzinfo object from pytz to return None for the offset, but I was able to hand construct such a tzinfo object locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be happy to tighten up this check if you're willing to open the issue. Sorry we didn't get it in v1.

# date is naive, re-interpret as system time zone
dt = dt.astimezone()

return dt
1 change: 1 addition & 0 deletions core/dbt/contracts/publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class PublicModel(dbtClassMixin, ManifestOrPublicNode):
# list of model unique_ids
public_node_dependencies: List[str] = field(default_factory=list)
generated_at: datetime = field(default_factory=datetime.utcnow)
deprecation_date: Optional[datetime] = None
Copy link
Contributor

Choose a reason for hiding this comment

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


@property
def is_latest_version(self) -> bool:
Expand Down
43 changes: 43 additions & 0 deletions core/dbt/events/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,49 @@ message UnpinnedRefNewVersionAvailableMsg {
UnpinnedRefNewVersionAvailable data = 2;
}

// I065
message DeprecatedModel {
string model_name = 1;
string model_version = 2;
string deprecation_date = 3;
}

message DeprecatedModelMsg {
EventInfo info = 1;
DeprecatedModel data = 2;
}

// I066
message UpcomingReferenceDeprecation {
string model_name = 1;
string ref_model_package = 2;
string ref_model_name = 3;
string ref_model_version = 4;
string ref_model_latest_version = 5;
string ref_model_deprecation_date = 6;
}

message UpcomingReferenceDeprecationMsg {
EventInfo info = 1;
UpcomingReferenceDeprecation data = 2;
}

// I067
message DeprecatedReference {
string model_name = 1;
string ref_model_package = 2;
string ref_model_name = 3;
string ref_model_version = 4;
string ref_model_latest_version = 5;
string ref_model_deprecation_date = 6;
}

message DeprecatedReferenceMsg {
EventInfo info = 1;
DeprecatedReference data = 2;
}


// M - Deps generation

// M001
Expand Down
56 changes: 56 additions & 0 deletions core/dbt/events/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,62 @@ def message(self) -> str:
return msg


class DeprecatedModel(WarnLevel):
def code(self):
return "I065"

def message(self) -> str:
version = ".v" + self.model_version if self.model_version else ""
return (
f"Model {self.model_name}{version} has passed its deprecation date of {self.deprecation_date}."
"This model should be disabled or removed."
peterallenwebb marked this conversation as resolved.
Show resolved Hide resolved
)


class UpcomingReferenceDeprecation(WarnLevel):
def code(self):
return "I066"

def message(self) -> str:
ref_model_version = ".v" + self.ref_model_version if self.ref_model_version else ""
msg = (
f"While compiling '{self.model_name}': Found a reference to {self.ref_model_name}{ref_model_version}, "
f"which is slated for deprecation on '{self.ref_model_deprecation_date}'. "
)

if self.ref_model_version and self.ref_model_version != self.ref_model_latest_version:
coda = (
f"A new version of '{self.ref_model_name}' is available. Try it out: "
f"{{{{ ref('{self.ref_model_package}', '{self.ref_model_name}', "
f"v='{self.ref_model_latest_version}') }}}}."
)
msg = msg + coda

return msg


class DeprecatedReference(WarnLevel):
def code(self):
return "I067"

def message(self) -> str:
ref_model_version = ".v" + self.ref_model_version if self.ref_model_version else ""
msg = (
f"While compiling '{self.model_name}': Found a reference to {self.ref_model_name}{ref_model_version}, "
f"which was deprecated on '{self.ref_model_deprecation_date}'. "
)

if self.ref_model_version and self.ref_model_version != self.ref_model_latest_version:
coda = (
f"A new version of '{self.ref_model_name}' is available. Migrate now: "
f"{{{{ ref('{self.ref_model_package}', '{self.ref_model_name}', "
f"v='{self.ref_model_latest_version}') }}}}."
)
msg = msg + coda

return msg


# =======================================================
# M - Deps generation
# =======================================================
Expand Down
878 changes: 445 additions & 433 deletions core/dbt/events/types_pb2.py

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def _create_parsetime_node(
language = ModelLanguage.python
config.add_config_call({"materialized": "table"})
else:
# this is not ideal but we have a lot of tests to adjust if don't do it
# this is not ideal, but we have a lot of tests to adjust if don't do it
language = ModelLanguage.sql

dct = {
Expand All @@ -212,6 +212,7 @@ def _create_parsetime_node(
"config": self.config_dict(config),
"checksum": block.file.checksum.to_dict(omit_none=True),
}

dct.update(kwargs)
try:
return self.parse_from_dict(dct, validate=True)
Expand Down
Loading