The Wayback Machine - http://web.archive.org/web/20211005223636/https://github.com/apache/superset/pull/16903
Skip to content

/ superset Public

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

Jump to bottom

feat: add Firebolt DB engine spec #16903

Merged
merged 15 commits into from Oct 1, 2021
Merged

feat: add Firebolt DB engine spec #16903

merged 15 commits into from Oct 1, 2021

Conversation

@apurva-sigmoid
Copy link
Contributor

@apurva-sigmoid apurva-sigmoid commented Sep 29, 2021

SUMMARY

Add a DB engine spec for Firebolt.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screenshot 2021-09-29 at 10 51 42 PM

Screenshot 2021-09-29 at 10 52 57 PM

Screenshot 2021-09-29 at 10 58 18 PM

TESTING INSTRUCTIONS

Connected Superset (running in docker-compose) to Firebolt database using Firebolt-SQLAlchemy adapter. Added database, dataset, queried from SQL Lab and created charts.

Also tested all the time grains.

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
raghavSharmaSigmoid and others added 12 commits Sep 7, 2021
@raghavSharmaSigmoid
New branch from superset for integration with firebolt sqlalchemy ada…
02a4f80 
…pter
@raghavSharmaSigmoid
Added db_engine_spec file for Firebolt
296ac3d
@raghavSharmaSigmoid
Removed firebolt code from superset repo
21d5ab9
@raghavSharmaSigmoid
Deleted virtual env commit
027913f
@apurva-sigmoid
Adding time grain changes to firebolt.py
5d9d581
@apurva-sigmoid
Updated README.md
45c5072 
Added steps to install and run Superset with Firebolt SQLAlchemy Adapter
@apurva-sigmoid
Update README.md
5ed17c7 
Reduced installation steps. Using PyPi installation for adapter now
@apurva-sigmoid
Revert "Update README.md"
acee352 
This reverts commit 5ed17c7.
@apurva-sigmoid
Revert "Updated README.md"
62e82ab 
This reverts commit 45c5072.
@apurva-sigmoid
Merge branch 'apache:master' into firebolt-integration
1d3d38b
@apurva-sigmoid
added epoch methods, added test cases for firebolt db engine spec and…
86c2603 
… edited setup.py
@apurva-sigmoid
Merge remote-tracking branch 'origin/firebolt-integration' into fireb…
3c4ef66 
…olt-integration
@apurva-sigmoid apurva-sigmoid changed the title feat: add Firebolt DB engine spec feat: [WIP] add Firebolt DB engine spec Sep 29, 2021
@apurva-sigmoid apurva-sigmoid changed the title feat: [WIP] add Firebolt DB engine spec feat: add Firebolt DB engine spec Sep 30, 2021
@villebro
Copy link
Member

@villebro villebro commented Sep 30, 2021

@apurva-sigmoid this looks great, thanks for the contribution! While you're at it, would you mind adding Firebolt to the docs:

@apurva-sigmoid
Copy link
Contributor Author

@apurva-sigmoid apurva-sigmoid commented Sep 30, 2021

License not added. I will add license and raise PR again

@apurva-sigmoid
Added license to files
33309e2
@codecov
Copy link

@codecov codecov bot commented Sep 30, 2021 •
edited

Codecov Report

Merging #16903 (e246d28) into master (3d8cc15) will decrease coverage by 0.16%.
The diff coverage is 80.89%.

❗ Current head e246d28 differs from pull request most recent head 7b5d61f. Consider uploading reports for the commit 7b5d61f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16903      +/-   ##
==========================================
- Coverage   77.02%   76.85%   -0.17%     
==========================================
  Files        1021     1023       +2     
  Lines       54754    54925     +171     
  Branches     7470     7485      +15     
==========================================
+ Hits        42173    42214      +41     
- Misses      12335    12463     +128     
- Partials      246      248       +2
Flag Coverage Δ
hive ?
mysql 81.86% <90.47%> (+0.02%) ⬆️
postgres 81.92% <90.53%> (+0.02%) ⬆️
presto 81.80% <89.94%> (+0.01%) ⬆️
python 82.18% <90.53%> (-0.23%) ⬇️
sqlite 81.54% <89.88%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/SqlLab/App.jsx 0.00% <ø> (ø)
...nd/src/assets/stylesheets/less/cosmo/cosmoTheme.js 0.00% <ø> (ø)
...frontend/src/components/DatabaseSelector/index.tsx 95.00% <ø> (+2.50%) ⬆️
superset-frontend/src/components/Icons/index.tsx 100.00% <ø> (ø)
superset-frontend/src/explore/App.jsx 0.00% <ø> (ø)
...d/src/filters/components/Time/TimeFilterPlugin.tsx 86.66% <ø> (ø)
superset-frontend/src/theme.ts 0.00% <0.00%> (ø)
superset-frontend/src/utils/downloadAsImage.ts 41.37% <ø> (ø)
superset-frontend/src/views/CRUD/hooks.ts 47.23% <ø> (ø)
superset-frontend/src/views/menu.tsx 0.00% <0.00%> (ø)
... and 31 more

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 3d8cc15...7b5d61f. Read the comment docs.

@apurva-sigmoid
Added documentation for Firebolt-SQLAlchemy
01a1374
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 30, 2021
@apurva-sigmoid
Copy link
Contributor Author

@apurva-sigmoid apurva-sigmoid commented Sep 30, 2021

@villebro as suggested I have added Firebolt to the documentation

@apurva-sigmoid apurva-sigmoid changed the title feat: add Firebolt DB engine spec feat: [WIP] add Firebolt DB engine spec Sep 30, 2021
@apurva-sigmoid apurva-sigmoid changed the title feat: [WIP] add Firebolt DB engine spec feat: add Firebolt DB engine spec Sep 30, 2021
@apurva-sigmoid
Removed trailing whitespace
7b5d61f
villebro
villebro approved these changes Oct 1, 2021
View changes
Copy link
Member

@villebro villebro left a comment

LGTM, thanks for this!

@villebro villebro merged commit 420eff4 into apache:master Oct 1, 2021
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@villebro villebro

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@apurva-sigmoid @villebro @raghavSharmaSigmoid