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

Exclude tests for disabled models in compile stats #2026

Merged
merged 2 commits into from
Jan 21, 2020
Merged

Exclude tests for disabled models in compile stats #2026

merged 2 commits into from
Jan 21, 2020

Conversation

NiallRees
Copy link
Contributor

Fix #1804

Checks if the node is a test, and then if it is enabled.

All models enabled:

(env) [niallwoodward:~/dev/data-dbt]$ dbt compile                                      
Running with dbt=0.15.0-rc2
Found 328 models, 725 tests, 2 snapshots, 1 analyse, 378 macros, 3 operations, 5 seed files, 104 sources

One model disabled before fix (with two tests dependent on the disabled model):

(env) [niallwoodward:~/dev/data-dbt]$ dbt compile                                      
Running with dbt=0.14.1
Found 327 models, 735 tests, 2 snapshots, 1 analyse, 374 macros, 3 operations, 5 seed files, 104 sources

One model disabled after fix (with two tests dependent on the disabled model):

(env) [niallwoodward:~/dev/data-dbt]$ dbt compile                                      
Running with dbt=0.15.0-rc2
Found 327 models, 723 tests, 2 snapshots, 1 analyse, 378 macros, 3 operations, 5 seed files, 104 sources

@cla-bot
Copy link

cla-bot bot commented Dec 28, 2019

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @NiallRees

1 similar comment
@cla-bot
Copy link

cla-bot bot commented Dec 28, 2019

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @NiallRees

@NiallRees
Copy link
Contributor Author

CLA is signed @drewbanin

Copy link
Contributor

@drewbanin drewbanin 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 making this PR @NiallRees! It looks great!! I'm going to kick off the tests now.

@drewbanin
Copy link
Contributor

@cla-bot check

@cla-bot cla-bot bot added the cla:yes label Jan 6, 2020
@cla-bot
Copy link

cla-bot bot commented Jan 6, 2020

The cla-bot has been summoned, and re-checked this pull request!

for node_name, node in itertools.chain(
manifest.nodes.items(),
manifest.macros.items()):
if _node_enabled(node): stats[node.resource_type] += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the flake8 styleguide checker we use isn't happy about this line:

core/dbt/compilation.py:70:31: E701 multiple statements on one line (colon)

via https://circleci.com/gh/fishtown-analytics/dbt/19687?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Can you just split this out onto two lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - thanks

@NiallRees
Copy link
Contributor Author

It looks like the CI failed for an unrelated reason to the pr, are you able to rerun @drewbanin?

@drewbanin
Copy link
Contributor

@NiallRees this is great! Thanks very much for your contribution to dbt - this code will ship in dbt v0.16.0 (Barbara Gittings 🎉

@drewbanin drewbanin merged commit 38244bf into dbt-labs:dev/barbara-gittings Jan 21, 2020
@NiallRees
Copy link
Contributor Author

@NiallRees this is great! Thanks very much for your contribution to dbt - this code will ship in dbt v0.16.0 (Barbara Gittings

Cool thanks - first of many I hope

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.

2 participants