Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

Infer the node name from the unique_id #10

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

dbeatty10
Copy link

resolves #9

Description

Infer the node name from the unique_id (to support changes introduced in dbt 0.19.0).

The maintainers will need to determine how the version of this package will need to be bumped if this PR is accepted.

I'm guessing this entry from the CHANGELOG is where the breaking changes came from:

  • Rationalize run result status reporting and clean up artifact schema (#2493, #2943)

Pros

  • minimal set of changes to achieve passing tests

Cons

  • someone might have a more comprehensive vision for altering how names and attributes might work in 0.19.0+

Caveats

  • I didn't test this pull request against any dbt versions other than 0.19.0rc1 (or databases other than Postgres)
  • depending on compatibility with versions earlier than 0.19.0rc1, either the dbt-adapter-tests version will need to be bumped or a different strategy will need to be taken

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

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Thanks for this @dbeatty! At the end of the day, it's not too much of a hack, though I'd prefer to ultimately refactor this code to compare based on unique_id instead.

@kwigley Could you take a look when you get a chance? We'll also need to cut a new version of pytest-dbt-adapter for plugin maintainers who want to test against the v0.19.0 release candidate.

@@ -323,8 +335,8 @@ def step_run_results(self, sequence_item, tmpdir):

for result in results:
try:
node = result['node']
name = node['name']
unique_id = result['unique_id']
Copy link
Contributor

@jtcohen6 jtcohen6 Jan 4, 2021

Choose a reason for hiding this comment

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

Ideally we could write this in a way that works for pre-0.19 versions, too, with handling for either result.get('unique_id') or result.get('node', {}).get('unique_id').

Edit: Based on the necessary changes to attributes (switching fail to status), I'm less sure if there's a simple way we can rewrite this so it's backwards-compatible for pre-v0.19 versions. We may just need a new minor release that's compatible for v0.19-and-later versions only.

@kwigley kwigley merged commit cc9251e into dbt-labs:master Jan 7, 2021
@kwigley
Copy link
Contributor

kwigley commented Jan 7, 2021

thanks @dbeatty10! I went ahead and merged this in order to cut v0.4.0 👍

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

Successfully merging this pull request may close these issues.

Not fully compatible with dbt-core 0.19.0rc1
4 participants