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

Track deprecation warnings #2710

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Aug 17, 2020

resolves #2688

Description

  • Fire one event per distinct deprecation per invocation
  • For now, we're just tracking the deprecation name, it's easy to add more later

Validation

In the warehouse:

SE_CATEGORY SE_ACTION SE_LABEL SE_PROPERTY CONTEXTS COLLECTOR_TSTAMP
dbt deprecation f8fdf25e-82ff-4769-963e-15727b3c51b1 warn {"data": [{"data": {"deprecation_name": "dbt-project-yaml-v1"},"schema": "iglu:com.dbt/deprecation_warn/jsonschema/1-0-0"}],"schema": "iglu:com.snowplowanalytics.snowplow/contexts/jsonschema/1-0-1"} 2020-08-17 20:15:05
dbt deprecation f8fdf25e-82ff-4769-963e-15727b3c51b1 warn {"data": [{"data": {"deprecation_name": "models-key-mismatch"},"schema": "iglu:com.dbt/deprecation_warn/jsonschema/1-0-0"}],"schema": "iglu:com.snowplowanalytics.snowplow/contexts/jsonschema/1-0-1"} 2020-08-17 20:15:05

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR I don't think we need a test, but let me know if you disagree!
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@jtcohen6 jtcohen6 requested a review from beckjake August 17, 2020 20:36
@cla-bot cla-bot bot added the cla:yes label Aug 17, 2020
Copy link
Contributor

@beckjake beckjake left a comment

Choose a reason for hiding this comment

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

This looks good to me. In the absence of tests, we should test this out live at least once before we do a final release, to make sure we're getting the information.

@jtcohen6
Copy link
Contributor Author

Agreed!

Do you know what's up with the failing mypy test? Something about implicit import/re-export?

@@ -16,6 +16,11 @@ def name(self) -> str:
'name not implemented for {}'.format(self)
)

def track_deprecation_warn(self) -> None:
dbt.tracking.track_deprecation_warn({
Copy link
Contributor

@beckjake beckjake Aug 17, 2020

Choose a reason for hiding this comment

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

I think mypy is mad here because you don't import dbt.tracking at the top. The code works as-is for stupid python reasons because you imported dbt.exceptions, but I'm surprised flake8 accepted this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh. Ok cool I'll fix

@jtcohen6 jtcohen6 merged commit c29892e into dev/marian-anderson Aug 18, 2020
@jtcohen6 jtcohen6 deleted the feature/add-deprecation-tracking branch August 18, 2020 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

track deprecations
2 participants