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

replace instances of dbt.exceptions with exceptions #1569

Closed
drewbanin opened this issue Jun 25, 2019 · 4 comments · Fixed by #1609
Closed

replace instances of dbt.exceptions with exceptions #1569

drewbanin opened this issue Jun 25, 2019 · 4 comments · Fixed by #1609
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@drewbanin
Copy link
Contributor

Issue

Issue description

Some default implementations of dbt macros reference dbt.exceptions instead of exceptions, leading to errors like

'dict object' has no attribute 'exceptions'.

In many (all?) of these cases, the macro is raising a "not implemented" exception, so this is a low-impact bug, but one worth fixing for sure.

Results

dbt raises the incorrect error message

System information

The output of dbt --version:

0.13.1

Steps to reproduce

This only really happens when you try to build a new adapter.

  1. create a new adapter
  2. don't implement methods like list_relations_without_caching
  3. run dbt
@drewbanin drewbanin added the bug Something isn't working label Jun 25, 2019
@drewbanin drewbanin added this to the Louisa May Alcott milestone Jun 27, 2019
@drewbanin drewbanin added the good_first_issue Straightforward + self-contained changes, good for new contributors! label Jul 11, 2019
@aminamos
Copy link
Contributor

Hi @drewbanin, if I look at this, which branch should I fork from? Apologies if this was addressed in the contributing file and I missed it. Thank you!

@drewbanin
Copy link
Contributor Author

Hey @aminamos - our current development branch is dev/0.14.1. I'd branch off of there in your fork for the PR!

I'll have a think about how to better document this. Let me know if you have any questions as you dig into it!

@aminamos
Copy link
Contributor

I submitted an initial pull request with the fixed macros to make sure I am on the right track. I do have one question: should I update the Python files as well?

@drewbanin
Copy link
Contributor Author

Hey @aminamos - I'll follow up on this in the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants