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

Show more detailed feedback when pyodbc import fails #192

Merged
merged 7 commits into from
Aug 10, 2021

Conversation

JCZuurmond
Copy link
Collaborator

@JCZuurmond JCZuurmond commented Jul 14, 2021

Description

When pydobc can not be imported and the user specifies this connection method, we raise an error that the dbt-spark[ODBC] should be installed. However, it can also be that the package is installed, but that it still can not be imported. Then, the ImportError details useful information, like some ODBC image can not be found, which is resolved with installing unixodb-dev.

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
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Jul 14, 2021
@JCZuurmond JCZuurmond force-pushed the detail-import-error-pyodbc branch from bcbbdd4 to b98bc8b Compare July 14, 2021 11:12
Error chaining does not show the message in `dbt debug`. Therefore we
explicitly add the message to the dbt.exceptions.RunTimeException
Add to change log that we print the import error when pyodbc can not be imported
@JCZuurmond
Copy link
Collaborator Author

Do we want to test this? Did we have a test already to verify the runtime exception message?

@JCZuurmond JCZuurmond changed the title Use exception chaining to get more detailed feedback when pyodbc is not installed Show more detailed feedback when pyodbc is not installed Jul 15, 2021
@JCZuurmond JCZuurmond changed the title Show more detailed feedback when pyodbc is not installed Show more detailed feedback when pyodbc import fails Jul 16, 2021
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.

This is a definite improvement! Thanks so much @JCZuurmond.

I don't believe we have a test for the existing error message. It's a good idea; at the same time, we've found pyodbc to be hard enough to work around in CI environments, and this change at least feels like an uncontroversial step in the right direction.

@jtcohen6 jtcohen6 merged commit 1a5c260 into dbt-labs:master Aug 10, 2021
jtcohen6 pushed a commit that referenced this pull request Aug 10, 2021
* Use exception chaining to get more detailed feedback when pyodbc is not installed

* Remove pyodbc referenced before assignment

* Set back try except

* Add flake ignore

* Add error message to RunTimeException

Error chaining does not show the message in `dbt debug`. Therefore we
explicitly add the message to the dbt.exceptions.RunTimeException

* Update change log

Add to change log that we print the import error when pyodbc can not be imported

* Fix parenthesis in change log
leahwicz added a commit that referenced this pull request Sep 17, 2021
* Show more detailed feedback when pyodbc import fails (#192)

* Use exception chaining to get more detailed feedback when pyodbc is not installed

* Remove pyodbc referenced before assignment

* Set back try except

* Add flake ignore

* Add error message to RunTimeException

Error chaining does not show the message in `dbt debug`. Therefore we
explicitly add the message to the dbt.exceptions.RunTimeException

* Update change log

Add to change log that we print the import error when pyodbc can not be imported

* Fix parenthesis in change log

* Update changelog [skip ci]

* Update changelog [skip ci]

* Update changelog [skip ci]

* Add support for ODBC Server Side Parameters (#201)

* Add support for ODBC Server Side Parameters

* Update CHANGELOG

* Feature/able to retry all connections (#194)

* Code changes

* README changes

* Improve error message default

* Changelog

* Changelog corrections

* Restore accidental deletion

* Update dbt/adapters/spark/connections.py

Co-authored-by: Jeremy Cohen <jtcohen6@gmail.com>

* Add myself to Contributors

Co-authored-by: Jeremy Cohen <jtcohen6@gmail.com>

* fixed get_columns_in_relation for open source delta table (#207)

* fixed get_columns_in_relation for open source delta table

* fixed E501 linting error and added change log

* fix issue parsing structs (#204)

* fix issue parsing structs

* include contributor in changelog

* better error explanation

Co-authored-by: Jeremy Cohen <jeremy@fishtownanalytics.com>

* Add adapter unique_field (#211)

* Add adapter unique_field

* Fix flake8. Add changelog entry

* [Snyk] Fix for 2 vulnerabilities (#214)

* fix: requirements.txt to reduce vulnerabilities


The following vulnerabilities are fixed by pinning transitive dependencies:
- https://snyk.io/vuln/SNYK-PYTHON-SQLPARSE-1584201
- https://snyk.io/vuln/SNYK-PYTHON-THRIFT-474615

* Removing Thrift conflict with versions over 12

Co-authored-by: leahwicz <60146280+leahwicz@users.noreply.github.com>

* 0.21.0b1 Release

* Bumping version to 0.21.0b1

* Bumping to 0.21.0b2

* Bumping version to 0.21.0b2

Co-authored-by: Cor <jczuurmond@protonmail.com>
Co-authored-by: Jeremy Cohen <jeremy@fishtownanalytics.com>
Co-authored-by: Jethro Nederhof <jethro@jethron.id.au>
Co-authored-by: gregingenii <80900458+gregingenii@users.noreply.github.com>
Co-authored-by: Jeremy Cohen <jtcohen6@gmail.com>
Co-authored-by: Hariharan Banukumar <hariharan.banukumar@gmail.com>
Co-authored-by: Sergio <ingscc00@gmail.com>
Co-authored-by: Snyk bot <github+bot@snyk.io>
Co-authored-by: Ian Knox <ian.knox@fishtownanalytics.com>
Co-authored-by: Leah Antkiewicz <leah.antkiewicz@dbtlabs.com>
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