apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
62.8k stars 13.87k forks source link

feat: cancel impala query on stop #30412

Closed wugeer closed 2 weeks ago

wugeer commented 1 month ago

feat: cancel sqllab impala query on stop

SUMMARY

Currently, when executing an Impala query in SQL Lab, clicking the cancel button only cancels the query in Superset, but the query is not canceled on the Impala side, and the resources are not released. This commit resolves the issue by ensuring that when an Impala query is canceled in SQL Lab, the query is also canceled on the Impala side, achieving the goal of releasing resources.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

before impala_sql_lab_cancel_before.webm

after impala_cancel_query.webm

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 78.57143% with 6 lines in your changes missing coverage. Please review.

Project coverage is 83.92%. Comparing base (76d897e) to head (4c933e1). Report is 895 commits behind head on master.

Files with missing lines Patch % Lines
superset/db_engine_specs/base.py 50.00% 4 Missing :warning:
superset/db_engine_specs/impala.py 90.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #30412 +/- ## =========================================== + Coverage 60.48% 83.92% +23.43% =========================================== Files 1931 534 -1397 Lines 76236 38754 -37482 Branches 8568 0 -8568 =========================================== - Hits 46114 32524 -13590 + Misses 28017 6230 -21787 + Partials 2105 0 -2105 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hive](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.93% <39.28%> (-0.24%)` | :arrow_down: | | [javascript](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | | | [mysql](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.74% <39.28%> (?)` | | | [postgres](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.84% <39.28%> (?)` | | | [presto](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.39% <39.28%> (-0.41%)` | :arrow_down: | | [python](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.92% <78.57%> (+20.43%)` | :arrow_up: | | [sqlite](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.29% <39.28%> (?)` | | | [unit](https://app.codecov.io/gh/apache/superset/pull/30412/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.91% <78.57%> (+3.28%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rusackas commented 1 month ago

Only @villebro 's review can unblock this now... thanks to both of you for following up on this!

wugeer commented 3 weeks ago

@villebro Could you please take some time to review my code? Thanks a lot!

wugeer commented 2 weeks ago

Looking good, however, in the interest of legibility, I would prefer a slightly more explicit name for the new property.

@villebro have Done! :)