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-1740] [Regression] Warning for tests on disabled resources should be DebugLevel, not WarnLevel #6501

Closed
2 tasks done
jtcohen6 opened this issue Jan 3, 2023 · 1 comment · Fixed by #6556
Closed
2 tasks done
Assignees
Labels
bug Something isn't working logging regression
Milestone

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Jan 3, 2023

Is this a regression in a recent version of dbt-core?

  • I believe this is a regression in dbt-core functionality
  • I have searched the existing issues, and I could not find an existing issue for this regression

Current Behavior

Reprise of #4594

If you dbt parse in a project with lots of tests on disabled resources (which is quite common for packages with conditionally enabled models), you'll immediately see a whole bunch of not-very-helpful warnings cluttering stdout:

$ dbt parse
13:24:39  Running with dbt=1.4.0-b1
13:24:39  Start parsing.
13:24:39  Dependencies loaded
13:24:39  Partial parse save file not found. Starting full parse.
13:24:39  ManifestLoader created
...
13:24:58  [WARNING]: Test 'test.microsoft_ads_source.unique_stg_microsoft_ads__account_history_account_version_id.e0ba88702d' (models/stg_microsoft_ads.yml) depends on a node named 'stg_microsoft_ads__account_history' which is disabled
13:24:58  [WARNING]: Test 'test.microsoft_ads_source.not_null_stg_microsoft_ads__account_history_account_version_id.2406ee6fb9' (models/stg_microsoft_ads.yml) depends on a node named 'stg_microsoft_ads__account_history' which is disabled
13:24:58  [WARNING]: Test 'test.microsoft_ads_source.unique_stg_microsoft_ads__ad_group_history_ad_group_version_id.37082929f6' (models/stg_microsoft_ads.yml) depends on a node named 'stg_microsoft_ads__ad_group_history' which is disabled
...

This is still true information, and worth logging, but only at a debug level.

Expected/Previous Behavior

This warning should be debug-level only

Steps To Reproduce

  1. Create a project with a disabled model, and a test on that model
  2. dbt parse

Relevant log output

No response

Environment

- OS: macOS 12.4
- Python: 3.10.9
- dbt (working version): 1.3.1
- dbt (regression version): 1.4.0-b1

Which database adapter are you using with dbt?

No response

Additional Context

The simplest fix for this is switching the InvalidDisabledTargetInTestNode event type from WarnLevel to DebugLevel here:

class InvalidDisabledTargetInTestNode(WarnLevel, pt.InvalidDisabledTargetInTestNode):

I realize that's inconsistent with other warnings, so it's worth us adding an annotation to call it out.

Actually, it even looks like we have some explicit logic to opt out this warning from the warn_or_error pattern. So it's really just a debug-level message that wants to have the [WARNING] prefix.

if node.resource_type == NodeType.Test:
if disabled:
fire_event(
InvalidDisabledTargetInTestNode(
resource_type_title=node.resource_type.title(),
unique_id=node.unique_id,
original_file_path=node.original_file_path,
target_kind=target_kind,
target_name=target_name,
target_package=target_package if target_package else "",
)
)
else:
warn_or_error(
NodeNotFoundOrDisabled(
original_file_path=node.original_file_path,
unique_id=node.unique_id,
resource_type_title=node.resource_type.title(),
target_name=target_name,
target_kind=target_kind,
target_package=target_package if target_package else "",
disabled=str(disabled),
)
)

@jtcohen6 jtcohen6 added this to the v1.4 milestone Jan 3, 2023
@github-actions github-actions bot changed the title [Regression] Warning for tests on disabled resources should be DebugLevel, not WarnLevel [CT-1740] [Regression] Warning for tests on disabled resources should be DebugLevel, not WarnLevel Jan 3, 2023
@peterallenwebb
Copy link
Contributor

It looks like this is happening because we consolidated the InvalidRefInTestNode (a debug-level event) and InvalidDisabledSourceInTestNode (a warn-level event) into a single, new event called InvalidDisabledTargetInTestNode (warn-level). We could remedy the issue by changing the level of the new event to debug instead of warn, but that would mean we are making some events which were warn in 1.3 into debug in 1.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants