You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/11 03:36:36 UTC
[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
graceguo-supercat opened a new pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `5.17%`.
> The diff coverage is `25.80%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 60.25% -5.18%
==========================================
Files 804 380 -424
Lines 37942 24087 -13855
Branches 3561 0 -3561
==========================================
- Hits 24823 14513 -10310
+ Misses 13010 9574 -3436
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.25% <25.80%> (-0.98%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.01% <18.51%> (-1.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.22% <66.66%> (-0.46%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| ... and [454 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...ccae7f3](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487327314
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `4.66%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 60.75% -4.67%
==========================================
Files 804 382 -422
Lines 37942 24145 -13797
Branches 3561 0 -3561
==========================================
- Hits 24823 14669 -10154
+ Misses 13010 9476 -3534
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.75% <100.00%> (-0.48%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `86.29% <100.00%> (-1.39%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.49% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| ... and [458 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...7e52c2d](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `5.04%`.
> The diff coverage is `96.77%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 60.83% -5.05%
==========================================
Files 815 382 -433
Lines 38337 24167 -14170
Branches 3601 0 -3601
==========================================
- Hits 25257 14702 -10555
+ Misses 12978 9465 -3513
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.83% <96.77%> (-0.64%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `87.27% <0.00%> (-1.98%)` | :arrow_down: |
| ... and [435 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5623cd64ca49c307df2cb5dc56d8981f7f68f081?el=desc) will **decrease** coverage by `4.52%`.
> The diff coverage is `97.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.84% 61.32% -4.53%
==========================================
Files 815 382 -433
Lines 38335 24163 -14172
Branches 3601 0 -3601
==========================================
- Hits 25243 14817 -10426
+ Misses 12990 9346 -3644
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `61.32% <97.14%> (-0.15%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.54% <0.00%> (-8.75%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.61% <0.00%> (-0.14%)` | :arrow_down: |
| ... and [431 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [5623cd6...8b1961b](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5623cd64ca49c307df2cb5dc56d8981f7f68f081?el=desc) will **decrease** coverage by `4.33%`.
> The diff coverage is `97.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.84% 61.51% -4.34%
==========================================
Files 815 382 -433
Lines 38335 24163 -14172
Branches 3601 0 -3601
==========================================
- Hits 25243 14863 -10380
+ Misses 12990 9300 -3690
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `61.51% <97.14%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [...end/src/explore/components/ControlPanelSection.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxTZWN0aW9uLmpzeA==) | | |
| [...tend/src/dashboard/containers/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | | |
| [...hboard/components/menu/BackgroundStyleDropdown.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL21lbnUvQmFja2dyb3VuZFN0eWxlRHJvcGRvd24uanN4) | | |
| [superset-frontend/src/components/ModalTrigger.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTW9kYWxUcmlnZ2VyLmpzeA==) | | |
| [...rontend/src/components/ListView/CardSortSelect.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvQ2FyZFNvcnRTZWxlY3QudHN4) | | |
| [...tend/src/views/CRUD/data/database/DatabaseList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZUxpc3QudHN4) | | |
| ... and [425 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [5623cd6...8b1961b](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489920646
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
correct. Dashboard is in synchronized mode, there is no query id passed from query engine to dashboard.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487328458
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.40%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.47% -4.41%
==========================================
Files 815 815
Lines 38337 38364 +27
Branches 3601 3601
==========================================
- Hits 25257 23586 -1671
- Misses 12978 14592 +1614
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.32% <97.14%> (-0.15%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [175 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...313a3e9](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488969833
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
+ self, dashboard_id: int
+ ) -> FlaskResponse:
+ if is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"):
+ username = g.user.username if g.user else None
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
+ slices = dashboard.slices
+ datasource_ids = set()
+ database_ids = set()
+
+ # find databases for all charts in a given dashboard
+ # stop pending query is only available for certain database(s)
+ for slc in slices:
Review comment:
moved to util function, and added unit test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r499838550
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
Review comment:
While the `dashboard` namespace was considered, the main use case for this endpoint is to terminate pending chart data queries, hence the `chart` namespace. The signature will likely be broadened in the future, making it possible to cancel queries based on other parameters (`cache_key`, `slice_id` etc). This endpoint will likely change significantly when the async framework is fully implemented, and will be revisited then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488045020
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
+ self, dashboard_id: int
+ ) -> FlaskResponse:
+ if is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"):
+ username = g.user.username if g.user else None
Review comment:
Since this is wrapped by `has_access_api`, does that guarantee that the user object already exists?
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
Is this dangerous? I assume this means that anyone who steals a session cookie could constantly kill all that user's queries?
it's probably fine for internal deployments, but I wonder if there's a way we could do this without removing CSRF protections
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
instead of making this a `get` function, why not call this `allows_stop_pending_queries` and put it up around line 150?
Also, to be more explicit with the API, can you add the `stop_queries` function to this class too and make it pass?
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
Alternatively, if `stop_queries` was an empty function, you might be able to remove the `allows_stop_pending_queries` attribute altogether. I think the only reason we might need the database level allows attribute would be if you wanted to prevent the frontend from sending the request to stop queries. But that could be messy as dashboards can be based off multiple different databases.
So maybe remove the `allows` check completely and just make `stop_queries` an empty function here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `9.29%`.
> The diff coverage is `64.28%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 56.13% -9.30%
==========================================
Files 804 398 -406
Lines 37942 13066 -24876
Branches 3561 3340 -221
==========================================
- Hits 24823 7334 -17489
+ Misses 13010 5543 -7467
- Partials 109 189 +80
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `56.13% <64.28%> (+0.75%)` | :arrow_up: |
| #javascript | `?` | |
| #python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `54.44% <61.53%> (-30.27%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <100.00%> (ø)` | |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...uperset-frontend/src/utils/getClientErrorObject.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2dldENsaWVudEVycm9yT2JqZWN0LnRz) | `0.00% <0.00%> (-89.19%)` | :arrow_down: |
| [.../src/dashboard/components/FilterIndicatorGroup.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvckdyb3VwLmpzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...c/explore/components/controls/withVerification.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy93aXRoVmVyaWZpY2F0aW9uLmpzeA==) | `9.09% <0.00%> (-87.88%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rset-frontend/src/profile/components/Favorites.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvY29tcG9uZW50cy9GYXZvcml0ZXMudHN4) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [649 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...574e38d](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488982417
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
I know 2 APIs here that don't use CSRF token: log and stop_dashboard_queries, both of them use `sendbeacon` which **can't** send CSRF token. The advantage of sendbeacon vs regular POST is [here](https://benborgers.com/blog/navigator-sendbeacon): it doesn't need to wait response. And it's pretty common for sendbeacon call go without CSRF token.
CSRF token is used to prevent malicious site from executing some transaction, like move money from your bank account to mine :) I feel kill other ppl's queries when their dashboard is still loading, is not very dangerous.
But for `superset.charts.api.data` i am not sure why it is in the exempt list. But this is not related to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r492170641
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
wow nice, what about iframe chart?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491810718
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
1. there is no `markdown slice` any more, 1 and 2 are same case: no slices
Added test cases for no slice, 2 database and druid slice cases.
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
There is no `markdown slice` any more, 1 and 2 are same case: no slices
Added test cases for no slice, 2 database and druid slice cases.
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
There is no `markdown slice` any more, 1 and 2 are same case: no slices.
Added test cases for no slice, 2 database and druid slice cases.
##########
File path: superset/views/utils.py
##########
@@ -213,6 +213,33 @@ def get_datasource_info(
return datasource_id, datasource_type
+def get_database_ids(dashboard_id: int) -> List[int]:
+ """
+ Find all database ids used by a given dashboard
+
+ :param dashboard_id: The dashboard id
+ :returns: A list of database ids used by the given dashboard
+ """
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
+ slices = dashboard.slices
+ datasource_ids: Set[int] = set()
+ database_ids: Set[int] = set()
+
+ for slc in slices:
+ datasource = slc.datasource
+ if (
+ datasource
+ and datasource.type == "table"
Review comment:
only return database id for `table` type datasources.
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
`markup`, `iframe` and `separator` visualization types, are not real slices since they do not generate query, i converted them into dashboard component and retried these viz types.
https://github.com/apache/incubator-superset/pull/10590
https://github.com/apache-superset/superset-ui/pull/746
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `4.84%`.
> The diff coverage is `25.80%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 60.57% -4.85%
==========================================
Files 804 380 -424
Lines 37942 24098 -13844
Branches 3561 0 -3561
==========================================
- Hits 24823 14598 -10225
+ Misses 13010 9500 -3510
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.57% <25.80%> (-0.66%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.01% <18.51%> (-1.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.50% <66.66%> (-0.18%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| ... and [438 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...ccae7f3](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `0.15%`.
> The diff coverage is `85.36%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 65.72% -0.16%
==========================================
Files 815 815
Lines 38337 38358 +21
Branches 3601 3601
==========================================
- Hits 25257 25212 -45
- Misses 12978 13039 +61
- Partials 102 107 +5
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `56.24% <64.28%> (-0.67%)` | :arrow_down: |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.49% <96.29%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `81.11% <61.53%> (-8.31%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <100.00%> (ø)` | |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `84.91% <100.00%> (+0.60%)` | :arrow_up: |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `33.33% <0.00%> (-16.67%)` | :arrow_down: |
| [superset-frontend/src/reduxUtils.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMudHM=) | `70.88% <0.00%> (-8.87%)` | :arrow_down: |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `76.97% <0.00%> (-5.50%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.25% <0.00%> (-4.92%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...44085f0](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491076631
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
+ dash_id = world_health.id
+ database_ids = get_database_ids(dash_id)
+ assert len(database_ids) == 1
+
+ world_slice = (
Review comment:
what is `get_examples_db()`? I didn't find any existed usage for this function.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r498086479
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ def data_stop(self) -> Response:
+ """
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user.
+ ---
+ post:
+ description: >-
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user
+ requestBody:
+ description: >-
+ The dashboard id.
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/ChartDataStopSchema"
+ responses:
+ 200:
+ description: Pending dashboard queries terminated
+ content:
+ application/json:
+ schema:
+ type: object
Review comment:
We don't need `content` on a empty response, also see if HTTP 204 makes sense
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489711964
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
yep, custom templates:
https://github.com/apache/incubator-superset/blob/master/tests/superset_test_config.py#L100
cta schema:
https://github.com/apache/incubator-superset/blob/master/tests/celery_tests.py#L176
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-694649751
ping @bkyryliuk do you have other concerns? thanks for code review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.40%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.47% -4.41%
==========================================
Files 815 815
Lines 38337 38364 +27
Branches 3601 3601
==========================================
- Hits 25257 23585 -1672
- Misses 12978 14593 +1615
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.31% <97.14%> (-0.16%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [176 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...313a3e9](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488064738
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
+ self, dashboard_id: int
+ ) -> FlaskResponse:
+ if is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"):
+ username = g.user.username if g.user else None
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
+ slices = dashboard.slices
+ datasource_ids = set()
+ database_ids = set()
+
+ # find databases for all charts in a given dashboard
+ # stop pending query is only available for certain database(s)
+ for slc in slices:
Review comment:
this is a good candidate for the util function - get database ids from the dashboard
by the way it would be much more efficient if that logic could be done in the SQL itself
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491810718
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
1. there is no `markdown slice` any more, 1 and 2 are same case: no slices
Added test cases for no slice, 2 database and druid slice cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r493757347
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
The reason `superset.charts.api.data` is exempt is because it is only a POST due to having a large request payload that doesn't sit well with a GET.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491076631
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
+ dash_id = world_health.id
+ database_ids = get_database_ids(dash_id)
+ assert len(database_ids) == 1
+
+ world_slice = (
Review comment:
_what is `get_examples_db()`? I didn't find any existed usage for this function._
never mind...just found it :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491025773
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
let's tests some edge cases here:
1. there is no database behind it e.g. markdown only
2. there are not slices in the dashboard
3. there are 2 databases used for the dashboard
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488969416
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
+ self, dashboard_id: int
+ ) -> FlaskResponse:
+ if is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"):
+ username = g.user.username if g.user else None
Review comment:
agree. fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `8.84%`.
> The diff coverage is `64.28%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 56.57% -8.85%
==========================================
Files 804 398 -406
Lines 37942 13066 -24876
Branches 3561 3340 -221
==========================================
- Hits 24823 7392 -17431
+ Misses 13010 5490 -7520
- Partials 109 184 +75
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `56.57% <64.28%> (+1.20%)` | :arrow_up: |
| #javascript | `?` | |
| #python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `54.44% <61.53%> (-30.27%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <100.00%> (ø)` | |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...uperset-frontend/src/utils/getClientErrorObject.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2dldENsaWVudEVycm9yT2JqZWN0LnRz) | `0.00% <0.00%> (-89.19%)` | :arrow_down: |
| [.../src/dashboard/components/FilterIndicatorGroup.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0ZpbHRlckluZGljYXRvckdyb3VwLmpzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...c/explore/components/controls/withVerification.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy93aXRoVmVyaWZpY2F0aW9uLmpzeA==) | `9.09% <0.00%> (-87.88%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rset-frontend/src/profile/components/Favorites.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvY29tcG9uZW50cy9GYXZvcml0ZXMudHN4) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| ... and [650 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...574e38d](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488064883
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
+1 to this suggestion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489008925
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -153,16 +151,30 @@ class Dashboard extends React.PureComponent {
}
if (hasUnsavedChanges) {
- Dashboard.onBeforeUnload(true);
+ window.addEventListener('beforeunload', Dashboard.showUnsavedMessage);
+ } else {
+ window.removeEventListener('beforeunload', Dashboard.showUnsavedMessage);
+ }
+
+ if (this.canStopPendingQueries && isDashboardLoading(charts)) {
+ window.addEventListener('beforeunload', this.onStopPendingQueries);
} else {
- Dashboard.onBeforeUnload(false);
+ window.removeEventListener('beforeunload', this.onStopPendingQueries);
}
}
componentWillUnmount() {
window.removeEventListener('visibilitychange', this.onVisibilityChange);
}
+ onStopPendingQueries() {
Review comment:
ok. fixed method name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `5.08%`.
> The diff coverage is `96.77%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 60.79% -5.09%
==========================================
Files 815 382 -433
Lines 38337 24156 -14181
Branches 3601 0 -3601
==========================================
- Hits 25257 14686 -10571
+ Misses 12978 9470 -3508
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.79% <96.77%> (-0.68%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.49% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| ... and [441 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `85.36%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 65.81% -0.07%
==========================================
Files 815 815
Lines 38337 38358 +21
Branches 3601 3601
==========================================
- Hits 25257 25247 -10
- Misses 12978 13007 +29
- Partials 102 104 +2
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `56.69% <64.28%> (-0.22%)` | :arrow_down: |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.49% <96.29%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `81.11% <61.53%> (-8.31%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <100.00%> (ø)` | |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `84.91% <100.00%> (+0.60%)` | :arrow_up: |
| [superset-frontend/src/reduxUtils.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3JlZHV4VXRpbHMudHM=) | `70.88% <0.00%> (-8.87%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `62.60% <0.00%> (-2.57%)` | :arrow_down: |
| [superset/models/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `87.27% <0.00%> (-1.98%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `39.58% <0.00%> (-1.50%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...44085f0](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487327314
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489568133
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
@bkyryliuk do you have an exist example, what is `implementation in the test config`? thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `5.17%`.
> The diff coverage is `25.80%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 60.25% -5.18%
==========================================
Files 804 380 -424
Lines 37942 24087 -13855
Branches 3561 0 -3561
==========================================
- Hits 24823 14513 -10310
+ Misses 13010 9574 -3436
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.25% <25.80%> (-0.98%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.01% <18.51%> (-1.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.22% <66.66%> (-0.46%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| ... and [454 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...ccae7f3](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r492182125
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
`markup`, `iframe` and `separator` visualization types, are not real slices since they do not generate query, i converted them into dashboard component and retried these viz types.
https://github.com/apache/incubator-superset/pull/10590
https://github.com/apache-superset/superset-ui/pull/746
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r492170641
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
wow nice, what about iframe chart?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r498085128
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ def data_stop(self) -> Response:
+ """
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user.
+ ---
+ post:
+ description: >-
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user
+ requestBody:
+ description: >-
+ The dashboard id.
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/ChartDataStopSchema"
+ responses:
+ 200:
+ description: Pending dashboard queries terminated
+ content:
+ application/json:
+ schema:
+ type: object
+ 400:
+ $ref: '#/components/responses/400'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if request.is_json:
+ json_body = request.json
+ dashboard_id = json_body.get("dashboard_id")
+ if not dashboard_id:
+ return self.response(400, message="dashboard_id missing in body")
+ hook = current_app.config["STOP_DASHBOARD_PENDING_QUERIES_HOOK"]
+ hook(dashboard_id, g.user.username)
Review comment:
`@protect` or `@has_access` does not guarantee we a have a user, since it's possible for a public/non authenticated user to access this resource. It's probably very unlikely that a `Public` role has access to this resource, but now it's possible to set `PUBLIC_ROLE_LIKE = "Admin"`.
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
+ @event_logger.log_this
+ @protect()
+ @safe
+ @statsd_metrics
+ def data_stop(self) -> Response:
+ """
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user.
+ ---
+ post:
+ description: >-
+ Takes a dashboard id and tries to cancel all associated chart data requests
+ issued by the user
+ requestBody:
+ description: >-
+ The dashboard id.
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/ChartDataStopSchema"
+ responses:
+ 200:
+ description: Pending dashboard queries terminated
+ content:
+ application/json:
+ schema:
+ type: object
Review comment:
Can you add more detail on the data structure of the response?
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
Review comment:
optional: maybe it would be simpler to set this endpoint to `/data/stop/<id>` HTTP POST or PUT, no need for a payload
##########
File path: superset/config.py
##########
@@ -309,6 +313,8 @@ def _try_json_readsha( # pylint: disable=unused-argument
"TAGGING_SYSTEM": False,
"SQLLAB_BACKEND_PERSISTENCE": False,
"LISTVIEWS_DEFAULT_CARD_VIEW": False,
+ # stop pending queries when user close/reload dashboard in browser
+ "STOP_DASHBOARD_PENDING_QUERIES": False,
Review comment:
If this is behind a feature flag we can go even further and not register the endpoint at all. Take a look at: https://github.com/apache/incubator-superset/blob/master/superset/dashboards/api.py#L173
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488992178
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
in open source code base, `stop_queries` function is empty, nothing to test. airbnb and other companies can add their internal implementation to stop queries by dashboard_id, but this is not a standard Presto API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488982417
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
I know 2 APIs here that don't use CSRF token: log and stop_dashboard_queries, both of
they use `sendbeacon` which **can't** send CSRF token. The advantage of sendbeacon vs regular POST is [here](https://benborgers.com/blog/navigator-sendbeacon): it doesn't need response. And it's pretty common for sendbeacon call go without CSRF token.
CSRF token is used to prevent malicious site from executing some transaction, like move money from your bank account to mine :) I feel kill other ppl's queries when their dashboard is still loading, is not very dangerous.
But for `superset.charts.api.data` i am not sure why it is in the exempt list. But this is not related to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488992178
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
note, in open source code base, presto's `stop_queries` function is empty, nothing to test. airbnb and other companies can add their internal implementation to stop queries by dashboard_id, but this is not a standard Presto API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] stale[bot] commented on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-751238759
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5623cd64ca49c307df2cb5dc56d8981f7f68f081?el=desc) will **decrease** coverage by `4.33%`.
> The diff coverage is `97.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.84% 61.51% -4.34%
==========================================
Files 815 382 -433
Lines 38335 24163 -14172
Branches 3601 0 -3601
==========================================
- Hits 25243 14863 -10380
+ Misses 12990 9300 -3690
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `61.51% <97.14%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [...perset-frontend/src/dashboard/util/findParentId.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2ZpbmRQYXJlbnRJZC5qcw==) | | |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | | |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | | |
| [...explore/components/controls/SelectAsyncControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9TZWxlY3RBc3luY0NvbnRyb2wuanN4) | | |
| [superset-frontend/src/views/CRUD/data/common.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9jb21tb24udHM=) | | |
| [...perset-frontend/src/explore/components/Control.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLmpzeA==) | | |
| ... and [425 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [5623cd6...8b1961b](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489891167
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
curious if there is an interest to have in in superset as well e.g. use smth like:
```CALL system.runtime.kill_query('20151207_215727_00146_tx3nr');```
https://github.com/prestodb/presto/issues/1515
However a challenge here would be tracking all queries and there ids
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489007214
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
airbnb's internal API accept username and dashboard_id :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.40%`.
> The diff coverage is `84.44%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.47% -4.41%
==========================================
Files 815 815
Lines 38337 38360 +23
Branches 3601 3601
==========================================
- Hits 25257 23582 -1675
- Misses 12978 14592 +1614
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.31% <96.77%> (-0.16%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [175 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489145325
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
it would be nice to have stop_queries implementation in the test config.
It will help in 2 ways:
1. be a safeguard from open source contributions not to break it
2. will work as an example for the companies willing to try it out
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.13%`.
> The diff coverage is `57.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.74% -4.14%
==========================================
Files 815 433 -382
Lines 38337 14193 -24144
Branches 3601 3601
==========================================
- Hits 25257 8764 -16493
+ Misses 12978 5243 -7735
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [542 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...7b4373f](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491811806
##########
File path: superset/views/utils.py
##########
@@ -213,6 +213,33 @@ def get_datasource_info(
return datasource_id, datasource_type
+def get_database_ids(dashboard_id: int) -> List[int]:
+ """
+ Find all database ids used by a given dashboard
+
+ :param dashboard_id: The dashboard id
+ :returns: A list of database ids used by the given dashboard
+ """
+ dashboard = db.session.query(Dashboard).filter_by(id=dashboard_id).one()
+ slices = dashboard.slices
+ datasource_ids: Set[int] = set()
+ database_ids: Set[int] = set()
+
+ for slc in slices:
+ datasource = slc.datasource
+ if (
+ datasource
+ and datasource.type == "table"
Review comment:
only return database id for `table` type datasources.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.70%`.
> The diff coverage is `84.44%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.17% -4.71%
==========================================
Files 815 815
Lines 38337 38360 +23
Branches 3601 3601
==========================================
- Hits 25257 23466 -1791
- Misses 12978 14708 +1730
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `60.83% <96.77%> (-0.64%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [178 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489920646
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
correct. Dashboard is running in synchronized mode, there is no query id passed from query engine to dashboard.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491026344
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
+ dash_id = world_health.id
+ database_ids = get_database_ids(dash_id)
+ assert len(database_ids) == 1
+
+ world_slice = (
Review comment:
it is safe to assume that examples_db is behind the chart, you can do
```
assert database_ids == [get_examples_db().id]
```
instead
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r493763393
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
Would it be possible to introduce a hook here that can either link a database or a db engine spec to a function that handles the query termination? Something similar to what `JINJA_CONTEXT_ADDONS` or `CUSTOM_TEMPLATE_PROCESSORS` does in `config.py`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `5.42%`.
> The diff coverage is `96.77%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 60.45% -5.43%
==========================================
Files 815 382 -433
Lines 38337 24156 -14181
Branches 3601 0 -3601
==========================================
- Hits 25257 14603 -10654
+ Misses 12978 9553 -3425
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.45% <96.77%> (-1.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `86.55% <66.66%> (-0.71%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.49% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| ... and [452 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489568133
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
do you have a exist example, what is `implementation in the test config`? thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488982417
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
I know 2 APIs here that don't use CSRF token here: log and stop_dashboard_queries, both of
they use `sendbeacon` which **can't** send CSRF token. The advantage of sendbeacon vs regular POST is [here](https://benborgers.com/blog/navigator-sendbeacon): it doesn't need response. And it's pretty common for sendbeacon call go without CSRF token.
CSRF token is used to prevent malicious site from executing some transaction, like move money from your bank account to mine :) I feel kill other ppl's queries when their dashboard is still loading, is not very dangerous.
But for `superset.charts.api.data` i am not sure why it is in the exempt list. But this is not related to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488990858
##########
File path: superset-frontend/src/dashboard/components/Dashboard.jsx
##########
@@ -153,16 +151,30 @@ class Dashboard extends React.PureComponent {
}
if (hasUnsavedChanges) {
- Dashboard.onBeforeUnload(true);
+ window.addEventListener('beforeunload', Dashboard.showUnsavedMessage);
+ } else {
+ window.removeEventListener('beforeunload', Dashboard.showUnsavedMessage);
+ }
+
+ if (this.canStopPendingQueries && isDashboardLoading(charts)) {
+ window.addEventListener('beforeunload', this.onStopPendingQueries);
} else {
- Dashboard.onBeforeUnload(false);
+ window.removeEventListener('beforeunload', this.onStopPendingQueries);
}
}
componentWillUnmount() {
window.removeEventListener('visibilitychange', this.onVisibilityChange);
}
+ onStopPendingQueries() {
Review comment:
I think it's the `beforeunload` event that triggers the `stopPendingQueries` action. So maybe we can just call this method `stopPendingQueries`?
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
Can we name this `stop_dashboard_queries` since it accepts `dashboard_id` as a required parameter? Also, would `user_id` be better suited for this API than `username`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489921651
##########
File path: tests/core_tests.py
##########
@@ -1231,6 +1231,22 @@ def test_get_column_names_from_metric(self):
"my_col"
]
+ @mock.patch.dict(
Review comment:
I added a test for `stop_dashboard_queries`. @bkyryliuk Do you think it make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489964787
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
if you find another solution, i am happy to learn :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5623cd64ca49c307df2cb5dc56d8981f7f68f081?el=desc) will **decrease** coverage by `4.52%`.
> The diff coverage is `97.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.84% 61.32% -4.53%
==========================================
Files 815 382 -433
Lines 38335 24163 -14172
Branches 3601 0 -3601
==========================================
- Hits 25243 14818 -10425
+ Misses 12990 9345 -3645
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `61.32% <97.14%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.54% <0.00%> (-8.75%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.61% <0.00%> (-0.14%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | | |
| ... and [430 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [5623cd6...8b1961b](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489568133
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
@bkyryliuk do you have an existed example, what is `implementation in the test config`? thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5623cd64ca49c307df2cb5dc56d8981f7f68f081?el=desc) will **decrease** coverage by `4.52%`.
> The diff coverage is `97.14%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.84% 61.32% -4.53%
==========================================
Files 815 382 -433
Lines 38335 24163 -14172
Branches 3601 0 -3601
==========================================
- Hits 25243 14818 -10425
+ Misses 12990 9345 -3645
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `61.32% <97.14%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.54% <0.00%> (-8.75%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.61% <0.00%> (-0.14%)` | :arrow_down: |
| [...rset-frontend/src/components/Icon/icon.stories.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvbi9pY29uLnN0b3JpZXMuanN4) | | |
| ... and [430 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [5623cd6...8b1961b](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488969112
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
thanks for the suggestion. fixed by remove `get_allow_stop_pending_queries` check, and add an empty `stop_queries`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r493757347
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
The reason `superset.charts.api.data` is exempt is because it is only a POST due to having a large request payload that doesn't sit well with a GET. Therefore it's not really a state changing POST, but a simulated GET.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] robdiciuccio commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r499826829
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
Review comment:
Curious why if this endpoint takes a `dashboard_id` value, does it live in the `charts` namespace?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488982417
##########
File path: superset/config.py
##########
@@ -172,7 +172,11 @@ def _try_json_readsha( # pylint: disable=unused-argument
WTF_CSRF_ENABLED = True
# Add endpoints that need to be exempt from CSRF protection
-WTF_CSRF_EXEMPT_LIST = ["superset.views.core.log", "superset.charts.api.data"]
+WTF_CSRF_EXEMPT_LIST = [
+ "superset.views.core.log",
+ "superset.charts.api.data",
+ "superset.views.core.stop_dashboard_queries",
Review comment:
I know 2 APIs here that don't use CSRF token: log and stop_dashboard_queries, both of
they use `sendbeacon` which **can't** send CSRF token. The advantage of sendbeacon vs regular POST is [here](https://benborgers.com/blog/navigator-sendbeacon): it doesn't need to wait response. And it's pretty common for sendbeacon call go without CSRF token.
CSRF token is used to prevent malicious site from executing some transaction, like move money from your bank account to mine :) I feel kill other ppl's queries when their dashboard is still loading, is not very dangerous.
But for `superset.charts.api.data` i am not sure why it is in the exempt list. But this is not related to this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a3e2e65121fadc8ca507c490eb5a3b26d8153041?el=desc) will **decrease** coverage by `4.84%`.
> The diff coverage is `25.80%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.42% 60.57% -4.85%
==========================================
Files 804 380 -424
Lines 37942 24098 -13844
Branches 3561 0 -3561
==========================================
- Hits 24823 14598 -10225
+ Misses 13010 9500 -3510
+ Partials 109 0 -109
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.57% <25.80%> (-0.66%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `73.01% <18.51%> (-1.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.50% <66.66%> (-0.18%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| ... and [438 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [a3e2e65...ccae7f3](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491025068
##########
File path: tests/core_tests.py
##########
@@ -1231,6 +1231,22 @@ def test_get_column_names_from_metric(self):
"my_col"
]
+ @mock.patch.dict(
+ "superset.extensions.feature_flag_manager._feature_flags",
+ {"STOP_DASHBOARD_PENDING_QUERIES": True},
+ clear=True,
+ )
+ def test_stop_dashboard_queries(self):
+ username = "admin"
+ self.login(username)
+ dashboard = self.get_dash_by_slug("births")
+ with mock.patch.object(BaseEngineSpec, "stop_queries") as mock_stop_queries:
+ resp = self.client.post(f"/superset/dashboard/{dashboard.id}/stop/")
+
+ self.assertTrue(is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"))
+ self.assertEqual(resp.status_code, 200)
+ mock_stop_queries.assert_called_once()
Review comment:
yeah this is great, let's add another assertion on the param the mock_stop_queries was called with
dunno if you should test druid as well here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491810718
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
There is no `markdown slice` any more, 1 and 2 are same case: no slices
Added test cases for no slice, 2 database and druid slice cases.
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
There is no `markdown slice` any more, 1 and 2 are same case: no slices.
Added test cases for no slice, 2 database and druid slice cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487328458
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r489920646
##########
File path: superset/db_engine_specs/base.py
##########
@@ -1001,3 +1001,13 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
logger.error(ex)
raise ex
return extra
+
+ @classmethod
+ def stop_queries(cls, username: str, dashboard_id: int) -> None:
Review comment:
correct. Dashboard is running in synchronized mode, there is no query id passed from query engine to dashboard. While in SQL lab, which is running in asynchronized mode, query id is saved into database, and celery Worker will update query status.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491025773
##########
File path: tests/utils_tests.py
##########
@@ -1162,3 +1164,15 @@ def test_get_form_data_token(self):
assert get_form_data_token({"token": "token_abcdefg1"}) == "token_abcdefg1"
generated_token = get_form_data_token({})
assert re.match(r"^token_[a-z0-9]{8}$", generated_token) is not None
+
+ def test_get_database_ids(self) -> None:
+ world_health = db.session.query(Dashboard).filter_by(slug="world_health").one()
Review comment:
let's tests some edge cases here:
1. there is no database behind it e.g. markdown only
2. there are not slices in the dashboard
3. there are 2 databases used for the dashboard
4. druid use case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r499938538
##########
File path: superset/charts/api.py
##########
@@ -503,6 +504,50 @@ def data(self) -> Response:
return response
+ @expose("/data/stop", methods=["POST"])
Review comment:
I feel there could be 2 use cases: terminate pending **query(s)** by dashboard_id, or terminate pending **query** by chart id. Currently airbnb has internal API to do 1), but i feel in the future (when the async framework is fully implemented), Superset should be able to do both.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `6.23%`.
> The diff coverage is `89.79%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 59.64% -6.24%
==========================================
Files 815 781 -34
Lines 38337 37238 -1099
Branches 3601 3346 -255
==========================================
- Hits 25257 22212 -3045
- Misses 12978 14841 +1863
- Partials 102 185 +83
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `56.27% <71.42%> (-0.63%)` | :arrow_down: |
| #javascript | `?` | |
| #python | `61.47% <97.14%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `58.88% <69.23%> (-30.53%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `100.00% <100.00%> (ø)` | |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/views/CRUD/data/savedquery/SavedQueryList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9zYXZlZHF1ZXJ5L1NhdmVkUXVlcnlMaXN0LnRzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| ... and [264 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...f29a5b0](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487327314
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `5.10%`.
> The diff coverage is `96.77%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 60.78% -5.11%
==========================================
Files 815 382 -433
Lines 38337 24167 -14170
Branches 3601 0 -3601
==========================================
- Hits 25257 14689 -10568
+ Misses 12978 9478 -3500
+ Partials 102 0 -102
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.78% <96.77%> (-0.69%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.49% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.16% <100.00%> (+0.86%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.16% <0.00%> (-12.50%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.85% <0.00%> (-11.44%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| ... and [437 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...00f6d93](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#issuecomment-691309530
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=h1) Report
> Merging [#10836](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.28%`.
> The diff coverage is `85.71%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10836 +/- ##
==========================================
- Coverage 65.88% 61.59% -4.29%
==========================================
Files 815 815
Lines 38337 38364 +27
Branches 3601 3601
==========================================
- Hits 25257 23631 -1626
- Misses 12978 14547 +1569
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.74% <57.14%> (-0.01%)` | :arrow_down: |
| #python | `61.50% <97.14%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/dashboard/components/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZC5qc3g=) | `76.66% <53.84%> (-12.75%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.09% <66.66%> (-0.17%)` | :arrow_down: |
| [superset-frontend/src/featureFlags.ts](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZlYXR1cmVGbGFncy50cw==) | `87.50% <100.00%> (-12.50%)` | :arrow_down: |
| [superset/config.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `90.42% <100.00%> (ø)` | |
| [superset/views/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.73% <100.00%> (+0.26%)` | :arrow_up: |
| [superset/views/utils.py](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvdXRpbHMucHk=) | `85.41% <100.00%> (+1.11%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [170 more](https://codecov.io/gh/apache/incubator-superset/pull/10836/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=footer). Last update [141ef4a...313a3e9](https://codecov.io/gh/apache/incubator-superset/pull/10836?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491025068
##########
File path: tests/core_tests.py
##########
@@ -1231,6 +1231,22 @@ def test_get_column_names_from_metric(self):
"my_col"
]
+ @mock.patch.dict(
+ "superset.extensions.feature_flag_manager._feature_flags",
+ {"STOP_DASHBOARD_PENDING_QUERIES": True},
+ clear=True,
+ )
+ def test_stop_dashboard_queries(self):
+ username = "admin"
+ self.login(username)
+ dashboard = self.get_dash_by_slug("births")
+ with mock.patch.object(BaseEngineSpec, "stop_queries") as mock_stop_queries:
+ resp = self.client.post(f"/superset/dashboard/{dashboard.id}/stop/")
+
+ self.assertTrue(is_feature_enabled("STOP_DASHBOARD_PENDING_QUERIES"))
+ self.assertEqual(resp.status_code, 200)
+ mock_stop_queries.assert_called_once()
Review comment:
yeah this is great, let's add another assertion on the param the mock_stop_queries was called with
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10836: [WIP] feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r487327314
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
##########
File path: superset/config.py
##########
@@ -628,11 +634,11 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# db configuration and a result of this function.
# mypy doesn't catch that if case ensures list content being always str
-ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[
- ["Database", "models.User"], List[str]
-] = lambda database, user: [
- UPLOADED_CSV_HIVE_NAMESPACE
-] if UPLOADED_CSV_HIVE_NAMESPACE else []
+ALLOWED_USER_CSV_SCHEMA_FUNC: Callable[["Database", "models.User"], List[str]] = (
Review comment:
`black` automatically formatted this section. not related with my PR.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that does not have stop queries API.
##########
File path: superset/db_engine_specs/base.py
##########
@@ -206,6 +206,10 @@ def is_db_column_type_match(
def get_allow_cost_estimate(cls, version: Optional[str] = None) -> bool:
return False
+ @classmethod
Review comment:
This check blocks requests to those databases that don't support stop queries function, or those companies that not implements stop queries API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r488063666
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
It may be useful to sync with @dpgaspar on using api v1.
Also please add some tests, we have presto on CI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull request #10836: feat: Stop pending queries when user close dashboard
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #10836:
URL: https://github.com/apache/incubator-superset/pull/10836#discussion_r491028573
##########
File path: superset/views/core.py
##########
@@ -1421,6 +1421,48 @@ def fave_slices( # pylint: disable=no-self-use
payload.append(dash)
return json_success(json.dumps(payload, default=utils.json_int_dttm_ser))
+ @api
+ @has_access_api
+ @event_logger.log_this
+ @expose("/dashboard/<dashboard_id>/stop/", methods=["POST"])
+ def stop_dashboard_queries( # pylint: disable=no-self-use
Review comment:
I think as I mentioned above, core apis are deprecated and new ones should be added here: https://github.com/apache/incubator-superset/blob/master/superset/dashboards/api.py @dpgaspar & @villebro would probably know more on this topic
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org