-
Notifications
You must be signed in to change notification settings - Fork 961
Fix - Presto SQLAlchemy dialect did not implement get_view_names #7190
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
Conversation
It doesn't seem lucky, the change causes test failure. Could you take a look? |
282d60e
to
18bbdf1
Compare
@pan3793, I have hopefully fixed it - I also extended the python range of tests to cover supported versions of Python, and bumped the package version in the hopes that a release could be made to PyPI when merged. |
Looking at failures, new python versions failing because of old packages. The previous versions of python still failing for same reason, I thought I had fixed it, checking... |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7190 +/- ##
=======================================
Coverage 0.00% 0.00%
=======================================
Files 695 696 +1
Lines 43433 43544 +111
Branches 5887 5891 +4
=======================================
- Misses 43433 43544 +111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
18bbdf1
to
9d2b4d1
Compare
9d2b4d1
to
b8deadc
Compare
@pan3793 - definitely no broken Python tests now, I removed the addition of current Python versions as they fail because dependencies are not up to date with Python > 3.10. So, just the changes required and tests now working. 🤞 More work than I was hoping for. |
Thanks, merged to master |
Why are the changes needed?
Presto SQLAlchemy dialect did not implement the
get_view_names
method and resulted in an exception when trying to inspect the schema. This was discovered in Superset repo whilst trying to update the pandas package which now makes a call toget_view_names
.How was this patch tested?
Very basic Python tests have been added here, but all the SQLAlchemy dialects in this repo would benefit from running the full SQLAlchemy dialect test suite instead of these bespoke tests.