Skip to content

Conversation

@guenp
Copy link
Contributor

@guenp guenp commented Jan 4, 2024

SUMMARY

Currently, when a user selects a DuckDB database, the schema dropdown shows multiple instances of main. This was an issue in the SQLAlchemy driver and has been fixed on the latest version of duckdb_engine.

BEFORE/AFTER SCREENSHOTS

Before:
Screenshot 2024-01-03 at 4 32 23 PM

After:
Screenshot 2024-01-03 at 4 34 02 PM

TESTING INSTRUCTIONS

  • Add a DuckDB database to Superset
  • Open SQLLab
  • Select the duckdb database
  • Click on the "schema" dropdown
  • Schema dropdown should contain unique db_name.schema_name items instead of many main schema duplicates

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6f6c37e) 69.14% compared to head (868b218) 69.14%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #26405      +/-   ##
==========================================
- Coverage   69.14%   69.14%   -0.01%     
==========================================
  Files        1946     1946              
  Lines       75990    75990              
  Branches     8479     8479              
==========================================
- Hits        52544    52543       -1     
- Misses      21267    21268       +1     
  Partials     2179     2179              
Flag Coverage Δ
hive 53.68% <ø> (ø)
mysql 78.07% <ø> (+0.02%) ⬆️
postgres 78.17% <ø> (ø)
presto 53.63% <ø> (ø)
python 82.85% <ø> (-0.01%) ⬇️
sqlite 76.83% <ø> (ø)
unit 55.80% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, but wondering if we should add an upper bound? I know you maintain the driver, so you're the best person to say if this is a valid concern or not..

@guenp guenp requested a review from villebro January 4, 2024 18:46
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM

@rusackas rusackas merged commit bba1b14 into apache:master Jan 4, 2024
@michael-s-molina michael-s-molina added the v3.1 Label added by the release manager to track PRs to be included in the 3.1 branch label Jan 9, 2024
michael-s-molina pushed a commit that referenced this pull request Jan 9, 2024
Co-authored-by: John Bodley <[email protected]>
(cherry picked from commit bba1b14)
josedev-union pushed a commit to Ortege-xyz/studio that referenced this pull request Jan 22, 2024
Co-authored-by: John Bodley <[email protected]>
(cherry picked from commit bba1b14)
@mistercrunch mistercrunch added 🍒 3.1.0 Cherry-picked to 3.1.0 🍒 3.1.1 Cherry-picked to 3.1.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Mar 8, 2024
@mistercrunch mistercrunch added 🍒 3.1.2 Cherry-picked to 3.1.2 🚢 4.0.0 First shipped in 4.0.0 labels Apr 17, 2024
@mistercrunch mistercrunch added the 🍒 3.1.3 Cherry-picked to 3.1.3 label Jul 2, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
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 v3.1 Label added by the release manager to track PRs to be included in the 3.1 branch 🍒 3.1.0 Cherry-picked to 3.1.0 🍒 3.1.1 Cherry-picked to 3.1.1 🍒 3.1.2 Cherry-picked to 3.1.2 🍒 3.1.3 Cherry-picked to 3.1.3 🚢 4.0.0 First shipped in 4.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants