Skip to content

Conversation

rad-pat
Copy link
Contributor

@rad-pat rad-pat commented Sep 4, 2025

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 to get_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.

@rad-pat rad-pat marked this pull request as ready for review September 4, 2025 09:14
@rad-pat rad-pat changed the title [WIP] Fix - Presto SQLAlchemy dialect did not implement get_view_names Fix - Presto SQLAlchemy dialect did not implement get_view_names Sep 4, 2025
@pan3793
Copy link
Member

pan3793 commented Sep 5, 2025

It doesn't seem lucky, the change causes test failure. Could you take a look?

@github-actions github-actions bot added the kind:infra license, community building, project builds, asf infra related, etc. label Sep 5, 2025
@rad-pat
Copy link
Contributor Author

rad-pat commented Sep 5, 2025

@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.

@rad-pat
Copy link
Contributor Author

rad-pat commented Sep 5, 2025

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-commenter
Copy link

codecov-commenter commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (ea75fa8) to head (c2d06f7).
⚠️ Report is 17 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot removed the kind:infra license, community building, project builds, asf infra related, etc. label Sep 5, 2025
@rad-pat
Copy link
Contributor Author

rad-pat commented Sep 7, 2025

@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.

@pan3793 pan3793 added this to the v1.11.0 milestone Sep 22, 2025
@pan3793 pan3793 closed this in 5771fdd Sep 22, 2025
@pan3793
Copy link
Member

pan3793 commented Sep 22, 2025

Thanks, merged to master

@pan3793 pan3793 reopened this Sep 22, 2025
@pan3793 pan3793 closed this in b992623 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants