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.

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

feat: support databend for superset #23308

Merged
merged 13 commits into from
Nov 2, 2023

Conversation

hantmac
Copy link
Contributor

@hantmac hantmac commented Mar 8, 2023

SUMMARY

Support new database - databend for superset

TESTING INSTRUCTIONS

pytest tests/unit_tests/db_engine_specs/test_databend.py

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

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.

Thanks for adding support for Databend! A few minor comments, other than that LGTM 👍

tests/unit_tests/db_engine_specs/test_databend.py Outdated Show resolved Hide resolved
tests/unit_tests/db_engine_specs/test_databend.py Outdated Show resolved Hide resolved
tests/unit_tests/db_engine_specs/test_databend.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #23308 (ccb676b) into master (9c54280) will decrease coverage by 1.52%.
The diff coverage is 54.96%.

❗ Current head ccb676b differs from pull request most recent head ab4ecab. Consider uploading reports for the commit ab4ecab to get more accurate results

@@            Coverage Diff             @@
##           master   #23308      +/-   ##
==========================================
- Coverage   69.00%   67.49%   -1.52%     
==========================================
  Files        1906     1908       +2     
  Lines       74149    73598     -551     
  Branches     8211     7975     -236     
==========================================
- Hits        51169    49676    -1493     
- Misses      20856    21874    +1018     
+ Partials     2124     2048      -76     
Flag Coverage Δ
hive 52.74% <47.68%> (-1.43%) ⬇️
javascript 53.74% <ø> (-2.10%) ⬇️
mysql 78.30% <47.68%> (-0.92%) ⬇️
postgres 78.36% <47.68%> (-0.96%) ⬇️
presto 52.67% <47.68%> (-1.41%) ⬇️
python 82.15% <54.96%> (-1.23%) ⬇️
sqlite 76.84% <47.68%> (-1.06%) ⬇️
unit 52.48% <54.96%> (-2.58%) ⬇️

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

Files Changed Coverage Δ
superset/db_engine_specs/databend.py 54.96% <54.96%> (ø)

... and 946 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hantmac
Copy link
Contributor Author

hantmac commented Mar 14, 2023

@villebro Fixed the comment and would like to help me fix this issue #23304?

@hantmac
Copy link
Contributor Author

hantmac commented Aug 10, 2023

Thanks for adding support for Databend! A few minor comments, other than that LGTM 👍

Hi @villebro , since this pr has been finished, how about merging this pr?

@villebro
Copy link
Member

Thanks for adding support for Databend! A few minor comments, other than that LGTM 👍

Hi @villebro , since this pr has been finished, how about merging this pr?

Hi @hantmac - I'm so sorry for dropping the ball on this PR. I've merged master on it to restart CI, and will re-review and merge if everything is ok today

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 the fantastic quality of this PR (tests, docs etc) ❤️

@villebro
Copy link
Member

@hantmac it seems there's quite a few linting errors here:
image

Would you be able to resolve them? If not I can pick this up and push it through next week.

@hantmac
Copy link
Contributor Author

hantmac commented Oct 30, 2023

@hantmac it seems there's quite a few linting errors here: image

Would you be able to resolve them? If not I can pick this up and push it through next week.

Thanks for your review! I will fix it as soon as possible.

@hantmac hantmac requested a review from villebro October 31, 2023 03:03
@villebro
Copy link
Member

@hantmac I restarted CI, will take a look tomorrow morning PST

@villebro villebro merged commit 5690946 into apache:master Nov 2, 2023
31 checks passed

_time_grain_expressions = {
None: "{col}",
"PT1M": "to_start_of_minute(TO_DATETIME({col}))",
Copy link
Member

Choose a reason for hiding this comment

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

Could you use superset.constants.TimeGrain?

TimeGrain.MINUTE: "to_start_of_minute(TO_DATETIME({col}))",

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 13, 2024
sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 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/XL 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants