Skip to content

fix: unambiguous trino driver - #18683

Merged
betodealmeida merged 1 commit into
apache:masterfrom
betodealmeida:trino_time_grains
Feb 12, 2022
Merged

betodealmeida merged 1 commit into
apache:masterfrom
betodealmeida:trino_time_grains

Conversation

@betodealmeida

Copy link
Copy Markdown
Member

SUMMARY

Both PyHive and sqlalchemy-trino declare a trino:// SQLAlchemy dialect. If you create a database in Superset with the trino:// SQLAlchemy URI and have both libraries installed the backend will use one the library randomly on every request, making it impossible to connect to a Trino database.

I created a PR upstream renaming the PyHive dialect to trino+pyhive:// (dropbox/PyHive#428), so that trino:// resolves unambiguously. In the meantime, this PR adds a trinonative:// entry pointing directly to the sqlalchemy-trino dialect, so that users with both libraries installed can connect to Trino.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  1. Connect to a Trino DB using the trinonative:// scheme.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Comment thread setup.py
entry_points={"console_scripts": ["superset=superset.cli.main:superset"]},
entry_points={
"console_scripts": ["superset=superset.cli.main:superset"],
"sqlalchemy.dialects": ["trinonative = sqlalchemy_trino.dialect:TrinoDialect"],

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this works even if the sqlalchemy-trino library is not installed.

@codecov

codecov Bot commented Feb 11, 2022 •

Copy link
Copy Markdown

Codecov Report

Merging #18683 (85f43bc) into master (a7d505d) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 85f43bc differs from pull request most recent head 565c59d. Consider uploading reports for the commit 565c59d to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18683      +/-   ##
==========================================
- Coverage   66.25%   66.19%   -0.06%     
==========================================
  Files        1604     1604              
  Lines       62807    62808       +1     
  Branches     6333     6333              
==========================================
- Hits        41613    41578      -35     
- Misses      19542    19578      +36     
  Partials     1652     1652              
Flag Coverage Δ
hive 52.14% <100.00%> (+<0.01%) ⬆️
mysql 81.33% <100.00%> (+<0.01%) ⬆️
postgres 81.38% <100.00%> (+<0.01%) ⬆️
presto ?
python 81.69% <100.00%> (-0.12%) ⬇️
sqlite 81.06% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/db_engine_specs/trino.py 70.79% <100.00%> (+0.26%) ⬆️
superset/db_engine_specs/presto.py 84.10% <0.00%> (-5.03%) ⬇️
superset/connectors/sqla/models.py 87.55% <0.00%> (-1.22%) ⬇️
superset/models/core.py 88.86% <0.00%> (-0.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7d505d...565c59d. Read the comment docs.

@betodealmeida
betodealmeida merged commit 40ab26d into apache:master Feb 12, 2022
@villebro

Copy link
Copy Markdown
Member

Nice! 👍

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 First shipped in 1.5.0 labels Mar 13, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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/XS 🚢 1.5.0 First shipped in 1.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants