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 2021/11/12 16:43:37 UTC
[GitHub] [superset] ofekisr opened a new pull request #17425: refactor(ChartDataCommand): into two separate commands
ofekisr opened a new pull request #17425:
URL: https://github.com/apache/superset/pull/17425
## **Background**
When we have worked on #16991 we wanted to test the new functionalities in concrete and accurate unittest.
All chartData flows and its components are too couple to superset so it is impossible to create unittests.
The flows are not testable and so many components do not meet the very important principle SRP and the code became so dirty
So I've started to refactor it (#17344 ) but many changes were added and it was hard to review so I decided to split those changes into small PRs so will be easier to follow
this is the fifth PR in a sequence of PRs to meet these
## PR description
1. Moves the commands module to be under charts.data package
2. Retrieving data and creating retrieve data job are two different responsibilities, so they should be in separate commands so the PR split it.
### Test plans
There are no logic changes so new tests are not required
## Previous PRs
1. #17399
2. #17400
3. #17405
4. #17407
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] [superset] codecov[bot] commented on pull request #17425: refactor(ChartDataCommand): into two separate commands
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17425:
URL: https://github.com/apache/superset/pull/17425#issuecomment-968373216
# [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17425](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25da471) into [master](https://codecov.io/gh/apache/superset/commit/d8851c9a893aa2b231f2634d9d083e8badc0601f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d8851c9) will **decrease** coverage by `0.00%`.
> The diff coverage is `94.11%`.
[](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17425 +/- ##
==========================================
- Coverage 76.83% 76.83% -0.01%
==========================================
Files 1041 1041
Lines 56069 56068 -1
Branches 7742 7742
==========================================
- Hits 43080 43079 -1
Misses 12731 12731
Partials 258 258
```
| Flag | Coverage Δ | |
|---|---|---|
| mysql | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| postgres | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| python | `82.04% <94.11%> (-0.01%)` | :arrow_down: |
| sqlite | `81.63% <94.11%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/charts/data/api.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvYXBpLnB5) | `88.57% <85.71%> (+0.08%)` | :arrow_up: |
| [superset/charts/data/commands.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvY29tbWFuZHMucHk=) | `97.87% <100.00%> (ø)` | |
| [superset/tasks/async\_queries.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3MvYXN5bmNfcXVlcmllcy5weQ==) | `96.96% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d8851c9...25da471](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] [superset] amitmiran137 merged pull request #17425: refactor(ChartDataCommand): into two separate commands
Posted by GitBox <gi...@apache.org>.
amitmiran137 merged pull request #17425:
URL: https://github.com/apache/superset/pull/17425
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] [superset] codecov[bot] edited a comment on pull request #17425: refactor(ChartDataCommand): into two separate commands
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17425:
URL: https://github.com/apache/superset/pull/17425#issuecomment-968373216
# [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17425](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25da471) into [master](https://codecov.io/gh/apache/superset/commit/d8851c9a893aa2b231f2634d9d083e8badc0601f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d8851c9) will **decrease** coverage by `0.00%`.
> The diff coverage is `94.11%`.
[](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17425 +/- ##
==========================================
- Coverage 76.83% 76.83% -0.01%
==========================================
Files 1041 1041
Lines 56069 56068 -1
Branches 7742 7742
==========================================
- Hits 43080 43079 -1
Misses 12731 12731
Partials 258 258
```
| Flag | Coverage Δ | |
|---|---|---|
| mysql | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| postgres | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| python | `82.04% <94.11%> (-0.01%)` | :arrow_down: |
| sqlite | `81.63% <94.11%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/charts/data/api.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvYXBpLnB5) | `88.57% <85.71%> (+0.08%)` | :arrow_up: |
| [superset/charts/data/commands.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvY29tbWFuZHMucHk=) | `97.87% <100.00%> (ø)` | |
| [superset/tasks/async\_queries.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3MvYXN5bmNfcXVlcmllcy5weQ==) | `96.96% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d8851c9...25da471](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] [superset] codecov[bot] edited a comment on pull request #17425: refactor(ChartDataCommand): into two separate commands
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17425:
URL: https://github.com/apache/superset/pull/17425#issuecomment-968373216
# [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17425](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (25da471) into [master](https://codecov.io/gh/apache/superset/commit/d8851c9a893aa2b231f2634d9d083e8badc0601f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d8851c9) will **decrease** coverage by `0.00%`.
> The diff coverage is `94.11%`.
[](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17425 +/- ##
==========================================
- Coverage 76.97% 76.97% -0.01%
==========================================
Files 1041 1041
Lines 56069 56068 -1
Branches 7742 7742
==========================================
- Hits 43157 43156 -1
Misses 12654 12654
Partials 258 258
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.52% <94.11%> (-0.01%)` | :arrow_down: |
| mysql | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| postgres | `81.95% <94.11%> (-0.01%)` | :arrow_down: |
| python | `82.31% <94.11%> (-0.01%)` | :arrow_down: |
| sqlite | `81.63% <94.11%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/charts/data/api.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvYXBpLnB5) | `88.57% <85.71%> (+0.08%)` | :arrow_up: |
| [superset/charts/data/commands.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2RhdGEvY29tbWFuZHMucHk=) | `97.87% <100.00%> (ø)` | |
| [superset/tasks/async\_queries.py](https://codecov.io/gh/apache/superset/pull/17425/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3MvYXN5bmNfcXVlcmllcy5weQ==) | `96.96% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d8851c9...25da471](https://codecov.io/gh/apache/superset/pull/17425?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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