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

[CT-2461] Support deprecating models #7433

Closed
Tracked by #7372
MichelleArk opened this issue Apr 21, 2023 · 1 comment · Fixed by #7562
Closed
Tracked by #7372

[CT-2461] Support deprecating models #7433

MichelleArk opened this issue Apr 21, 2023 · 1 comment · Fixed by #7562
Assignees

Comments

@MichelleArk
Copy link
Contributor

MichelleArk commented Apr 21, 2023

Acceptance Criteria

Parsing:

  • Any model (not just versioned ones) can provide a non-null deprecation_date as a top-level property (not under config). Other node types (e.g. seeds, snapshots, etc) can not.
    • Unparsed deprecation_date is a string - formatted as a date, optionally with a timezone offset (e.g. '2020-10-23+00:00' or equivalently '2020-10-23'. Parsed representation of deprecation_date on ModelNode should be a datetime (parsed from one of the two supported formats)
    • Models with deprecation_date values that are set to a past date (as compared to datetime.now(pytz.utc)) should raise a DeprecatedModel warning.

ref behaviour:

  • parse-time references to a model that has a deprecation_date set to a:
    • future date (as compared to datetime.now(pytz.utc)) -> raise an UpcomingDeprecationReference warning, containing both the deprecation date and its relative time
    • past date (as compared to datetime.now(pytz.utc)) -> raise a DeprecatedReference warning
    • (We'd want these two to be separate so they can be configured for error-promotion via WARN_ERROR_OPTIONS individually)

Warning messages:

  • DeprecatedModel (warning when parsing a project that defines deprecated model(s)): [WARNING] Model {model_name}[.v{version} if versioned] has passed its deprecation date of {deprecation_date}. This model should be disabled or removed.
  • UpcomingDeprecationReference (warning when referencing a model with a future deprecation date): [WARNING] While compiling '{this_model_name}': Found a reference to {ref_model_name}[.v{version} if versioned], which is slated for deprecation on '{ref_model_deprecation_date}'. [if versioned: A new version of '{ref_model_name}' is available. Try it out: {{ ref('ref_model_package', 'ref_model_name', v='{ref_model_latest_version}') }}.
  • DeprecatedReference (warning when referencing a model with a past deprecation date): [WARNING] While compiling '{this_model_name}': Found a reference to {ref_model_name}[.v{version} if versioned], which was deprecated on '{ref_model_deprecation_date}'. [if versioned: A new version of '{ref_model_name}' is available. Migrate now: {{ ref('ref_model_package', 'ref_model_name', v='{ref_model_latest_version}') }}.

For inspiration: UnpinnedRefNewVersionAvailable. I would also be happy if we cleaned up that "info"-level log, perhaps by moving it to an end-of-run summary. We don't have a great way to surface & summarize warnings on the CLI currently—and facilitating this type of cross-team communication is the feature!

@github-actions github-actions bot changed the title Support deprecating models [CT-2461] Support deprecating models Apr 21, 2023
@jtcohen6
Copy link
Contributor

jtcohen6 commented Apr 21, 2023

As discussed at length over in #6736, there are degrees of strictness that we could pursue here. We could choose to be more strict by default:

  • Models past their deprecation date aren't selected to run by default (producer)
  • Models past their deprecation date are considered disabled (producer) / raise an error on ref, rather than a warning (consumer)

Our approach so far has been to provide optionality in the primitives and strong guidance in their application. I'm inclined to do the same here, though knowing well that we run the risk of "warning" fatigue. If a deprecation date is approaching, and the model actually needs to stick around - maybe it should be the responsibility of its maintainer to admit defeat, and push back the deprecation_date.

In this case:

  • WARN_ERROR_OPTIONS does give us a mechanism whereby users can promote these warnings to actual runtime errors
  • We could encode a lot more such guidance in, e.g., dbt_project_evaluator — or other custom macros that raise compilation errors — if (e.g.) you have old model versions that don't have a deprecation_date.

So - I'm good with the proposal outlined above!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants