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

[database] Fix, test connection error message for module not found #9634

Merged
merged 2 commits into from
Apr 27, 2020

Conversation

dpgaspar
Copy link
Member

@dpgaspar dpgaspar commented Apr 23, 2020

CATEGORY

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

On MSSQL and probably using other drivers, if pymssql is missing testconn API response is "Unexpected error occurred, please check your logs for details" and should be "Could not load database driver: ..."

TEST PLAN

  • Create a database connection with: mssql+pymssql://someserver:1433/test
  • Check that the error reported is "ERROR: Could not load database driver: mssql+pymssql"

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

REVIEWERS

@willbarrett
Copy link
Member

Would it be possible to add a test for this case?

@villebro
Copy link
Member

Hmm, I wasn't able to reproduce with the test string something+dummy://someserver:9999/test. Were you testing a specific driver when this came up?

@dpgaspar
Copy link
Member Author

dpgaspar commented Apr 24, 2020

@villebro

Caught this while testing MSSQL, so used:
mssql+pymssql://sa:XXXXXXXXXX@localhost:1433/msdb

You can reproduce it by removing the pymssql from the virtualenv

Yet, you're right, my assumption is not totally correct, updated the PR description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 0.37.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants