Skip to content
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.

Sign up for GitHub

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

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

raghavSharmaCode and others added 12 commits September 7, 2021 11:55
@raghavSharmaCode
New branch from superset for integration with firebolt sqlalchemy ada…
02a4f80 
…pter
@raghavSharmaCode
Added db_engine_spec file for Firebolt
296ac3d
@raghavSharmaCode
Removed firebolt code from superset repo
21d5ab9
@raghavSharmaCode
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

@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

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

@codecov
Copy link

codecov bot commented Sep 30, 2021 •
edited
Loading

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.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 30, 2021
@apurva-sigmoid
Copy link
Contributor Author

@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
villebro
villebro approved these changes Oct 1, 2021
View reviewed changes
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, thanks for this!

@villebro villebro merged commit 420eff4 into apache:master Oct 1, 2021
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
@apurva-sigmoid @raghavSharmaCode
feat: add Firebolt DB engine spec (apache#16903)
f3b7815 
* New branch from superset for integration with firebolt sqlalchemy adapter

* Added db_engine_spec file for Firebolt

* Removed firebolt code from superset repo

* Deleted virtual env commit

* Adding time grain changes to firebolt.py

* Updated README.md

Added steps to install and run Superset with Firebolt SQLAlchemy Adapter

* Update README.md

Reduced installation steps. Using PyPi installation for adapter now

* Revert "Update README.md"

This reverts commit 5ed17c7.

* Revert "Updated README.md"

This reverts commit 45c5072.

* added epoch methods, added test cases for firebolt db engine spec and edited setup.py

* Added license to files

* Added documentation for Firebolt-SQLAlchemy

* Removed trailing whitespace

Co-authored-by: raghavsharma <[email protected]>
Co-authored-by: raghavSharmaSigmoid <[email protected]>
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
@apurva-sigmoid @raghavSharmaCode
feat: add Firebolt DB engine spec (apache#16903)
18a4903 
* New branch from superset for integration with firebolt sqlalchemy adapter

* Added db_engine_spec file for Firebolt

* Removed firebolt code from superset repo

* Deleted virtual env commit

* Adding time grain changes to firebolt.py

* Updated README.md

Added steps to install and run Superset with Firebolt SQLAlchemy Adapter

* Update README.md

Reduced installation steps. Using PyPi installation for adapter now

* Revert "Update README.md"

This reverts commit 5ed17c7.

* Revert "Updated README.md"

This reverts commit 45c5072.

* added epoch methods, added test cases for firebolt db engine spec and edited setup.py

* Added license to files

* Added documentation for Firebolt-SQLAlchemy

* Removed trailing whitespace

Co-authored-by: raghavsharma <[email protected]>
Co-authored-by: raghavSharmaSigmoid <[email protected]>
@siegfriedweber siegfriedweber mentioned this pull request Feb 9, 2022
Closed
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@villebro villebro villebro approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@apurva-sigmoid @villebro @mistercrunch @raghavSharmaCode