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

Tests folder is Included during os.walk Additionally with no Exception Handling during Model Queries #347

Open
z3z1ma opened this issue Sep 24, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@z3z1ma
Copy link
Contributor

z3z1ma commented Sep 24, 2021

Long title but the issue is interrelated.

The tests folder is included when building model catalog. This is a problem because tests are not manifest as tables in the database but they are still being added to models list and queried for while connected to the database. This cause a SQLAlchemy exception that the table does not exist and the program hard fails. In theory you can pass excluded folders: - tests
but that is suboptimal because everyone who uses data tests would need to do this.

I propose 2 things:

  1. Lets ignore some directories by default leveraging the dbt_project yml instead of this exclusion pattern here because it has both the issue of the tests folder not being excluded but also it is not resilient to users with non standard directories

image

  1. Lets add some basic exception handling during the loop that is building models and actually querying the database. Then we can decide if we want to fail gracefully or push past. Without that, users will be completely stuck if a model has not materialized or is dropped in a post run hook or who knows.

I'll put up a PR a little later today. LMK thoughts otherwise

@z3z1ma z3z1ma added the bug Something isn't working label Sep 24, 2021
@bastienboutonnet
Copy link
Member

Hi @z3z1ma makes sense to me.

Regarding 1:
I think we probably still want to have the exclusion pattern here, but reading it from the dbt_profile.yml as well as sugar_config.yml should not be mutually exclusive. In the end, I would read from all of those places and compose the right exclusion pattern. How does that sound to you?

Regarding 2: Absolutely agree, it's not something I encountered during development but I think it would make total sense. Do you have an idea of how you want to write that or do you want to spitball a few ideas?

PR totally welcome whenever you feel like it. I'm a bit snowed under these days so if you have time do go for it, if not I'll get around to it as soon as I can dive deeper into this again.

@bastienboutonnet bastienboutonnet changed the title [BUG] Tests folder is Included during os.walk Additionally with no Exception Handling during Model Queries Tests folder is Included during os.walk Additionally with no Exception Handling during Model Queries Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants