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/01/27 00:15:15 UTC
[GitHub] [superset] pkdotson opened a new pull request #12782: feat: [wip] reset metrics on dataset change
pkdotson opened a new pull request #12782:
URL: https://github.com/apache/superset/pull/12782
### SUMMARY
This pr will reset metrics controls when there is a change in dataset.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or 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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (0cbbce2) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.52%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 62.89% -3.53%
==========================================
Files 1022 1022
Lines 50106 50112 +6
Branches 5193 5206 +13
==========================================
- Hits 33283 31520 -1763
- Misses 16680 18381 +1701
- Partials 143 211 +68
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.70% <42.85%> (-0.04%)` | :arrow_down: |
| python | `63.68% <ø> (-0.39%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ore/components/controls/AnnotationLayerControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXJDb250cm9sLmpzeA==) | `8.33% <0.00%> (-73.10%)` | :arrow_down: |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `19.44% <0.00%> (-1.77%)` | :arrow_down: |
| [...ents/controls/FilterControl/AdhocFilterControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyQ29udHJvbC5qc3g=) | `58.20% <0.00%> (-5.43%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `75.00% <ø> (ø)` | |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `68.42% <ø> (-26.32%)` | :arrow_down: |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `70.58% <ø> (-23.53%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `39.58% <16.66%> (-29.47%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `58.77% <50.00%> (-21.85%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `82.65% <50.00%> (-3.31%)` | :arrow_down: |
| [...rc/explore/components/controls/CheckboxControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9DaGVja2JveENvbnRyb2wuanN4) | `82.35% <66.66%> (-3.37%)` | :arrow_down: |
| ... and [164 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (fce0b7d) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.04%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.38% -3.05%
==========================================
Files 1022 488 -534
Lines 50106 30143 -19963
Branches 5193 0 -5193
==========================================
- Hits 33283 19106 -14177
+ Misses 16680 11037 -5643
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.38% <ø> (-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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.59% <0.00%> (-3.27%)` | :arrow_down: |
| ... and [533 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] junlincc edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770122764
> I guess I got a little concerned when you said this is the end of this project:
The end state of this project **for the time being**. We really want to make incremental changes and iterate rapidly instead of holding off the progress because of factors that are out of control.
Back to the solution itself... I am going to respect the original feature request received from the forum for now. let's discuss more next week to explore different options along with other contentious topics. im sure we can reach consensus somehow :) @mihir174 @ktmud @villebro
----------------------------------------------------------------
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] [superset] zhaoyongjie merged pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged pull request #12782:
URL: https://github.com/apache/superset/pull/12782
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (0cbbce2) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `2.72%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.69% -2.73%
==========================================
Files 1022 488 -534
Lines 50106 30130 -19976
Branches 5193 0 -5193
==========================================
- Hits 33283 19192 -14091
+ Misses 16680 10938 -5742
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.69% <ø> (-0.38%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/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/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.22% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/reports/notifications/base.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2Jhc2UucHk=) | `95.00% <0.00%> (-0.46%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.98% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.44% <0.00%> (-0.15%)` | :arrow_down: |
| [superset/migrations/shared/security\_converge.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy9zaGFyZWQvc2VjdXJpdHlfY29udmVyZ2UucHk=) | `86.82% <0.00%> (-0.11%)` | :arrow_down: |
| ... and [528 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770002423
And FYI, one of the motivation behind [SIP-43](https://github.com/apache/superset/issues/9887) was to have the ability to retain control values when switching viz types:
> This not only greatly simplifies the code, but also makes it easier to switch between visualization types---all control values for Query, Transform, and even Columns can be easily retained.
I think it would make sense to extend that ability to datasource switch, too.
----------------------------------------------------------------
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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722211
I think it would be nice to retain metrics/columns with matching column names since a lot of times users would want to fix a broken chart or iterate on an existing one by replacing the datasource with a newer (and slightly updated) dataset.
I know this requirement exists because as a former Data Scientist, I've done similar operation in Tableau a lot.
----------------------------------------------------------------
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] [superset] junlincc commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770110781
> And FYI, one of the motivation behind [SIP-43](https://github.com/apache/superset/issues/9887) was to have the ability to retain control values when switching viz types:
>
> > This not only greatly simplifies the code, but also makes it easier to switch between visualization types---all control values for Query, Transform, and even Columns can be easily retained.
>
> I think it would make sense to extend that ability to datasource switch, too.
Right, that's the goal. In terms of timeline, it probably will happen naturally after Echarts migration and dynamic control that @villebro proposed are both completed. Well actually it's already happening in Time-series Echarts. Really appreciate your suggestions though!
since chart creation flow is still fairly broken, and the lack of consistency in control fields across different chart types, it makes more sense reseting to default when switching dataset for now.
@pkdotson please address @villebro's comment, then we are good to go. thanks you 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] [superset] junlincc edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770110781
> And FYI, one of the motivation behind [SIP-43](https://github.com/apache/superset/issues/9887) was to have the ability to retain control values when switching viz types:
>
> > This not only greatly simplifies the code, but also makes it easier to switch between visualization types---all control values for Query, Transform, and even Columns can be easily retained.
>
> I think it would make sense to extend that ability to datasource switch, too.
Right, that's the goal. In terms of timeline, it probably will happen naturally after Echarts migration and dynamic control that @villebro proposed. Well actually it's already happening in Time-series Echarts. Really appreciate your suggestions though!
since chart creation flow is still fairly broken, and the lack of consistency in control fields across different chart types, it makes more sense reseting to default when switching dataset for now.
@pkdotson please address @villebro's comment, then we are good to go. thanks you 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] [superset] pkdotson commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
pkdotson commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r569011801
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -123,6 +123,9 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
adhocMetricLabel: this.state.adhocMetric?.getDefaultLabel(),
});
}
+ if (prevProps.datasource !== this.props.datasource) {
Review comment:
This does refer's to the the datasource name. datasource is datasource name being passed down
----------------------------------------------------------------
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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722823
A better experience is probably to still keep the selected options, but highlight the invalid ones (like Tableau does).
----------------------------------------------------------------
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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770119750
I guess I got a little concerned when you said this is the end of this project:
```
end state of this project
- reset/clear/uncheck all query and customize control when dataset is changed
- In the change dataset warning msg, add a line to notify users all the current control settings will be cleared by changing dataset.
```
Rather than resetting everything to default, I think it makes more sense to just reset `SelectControl`, `MetricsControl` and `AdhocFilterControl` to their default state. Because if the end is to retain control values for other controls, we already don't have to do anything special about them. The select controls are trickier because you need to check column validity.
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (29a42e0) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **increase** coverage by `2.53%`.
> The diff coverage is `59.25%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
+ Coverage 66.42% 68.95% +2.53%
==========================================
Files 1022 1026 +4
Lines 50106 48856 -1250
Branches 5193 5257 +64
==========================================
+ Hits 33283 33689 +406
+ Misses 16680 15033 -1647
+ Partials 143 134 -9
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.83% <59.25%> (+0.35%)` | :arrow_up: |
| javascript | `61.78% <48.14%> (+0.05%)` | :arrow_up: |
| python | `67.32% <ø> (+3.25%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `41.66% <0.00%> (+20.45%)` | :arrow_up: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `94.73% <ø> (ø)` | |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `94.11% <ø> (ø)` | |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `90.11% <ø> (+4.15%)` | :arrow_up: |
| [.../src/explore/components/ControlPanelsContainer.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxzQ29udGFpbmVyLmpzeA==) | `86.73% <37.50%> (-4.38%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `79.85% <50.00%> (-0.77%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `68.75% <66.66%> (-0.30%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `100.00% <100.00%> (+25.00%)` | :arrow_up: |
| [.../src/explore/components/controls/SelectControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9TZWxlY3RDb250cm9sLmpzeA==) | `93.69% <100.00%> (+0.23%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [150 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...29a42e0](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] junlincc commented on pull request #12782: feat: [wip] reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-768040007
end state of this project
1. reset/clear/uncheck all query and customize control when dataset is changed
2. In the change dataset warning msg, add a line to notify users all the current control settings will be cleared by changing dataset.
----------------------------------------------------------------
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] [superset] codecov-io commented on pull request #12782: feat: [wip] reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (2825ce0) into [master](https://codecov.io/gh/apache/superset/commit/017f11f9d84ac88019a6c42eb65a67b617966e95?el=desc) (017f11f) will **increase** coverage by `0.25%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
+ Coverage 63.13% 63.39% +0.25%
==========================================
Files 1022 488 -534
Lines 50032 30117 -19915
Branches 4915 0 -4915
==========================================
- Hits 31587 19092 -12495
+ Misses 18245 11025 -7220
+ Partials 200 0 -200
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `?` | |
| python | `63.39% <ø> (-0.71%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-6.28%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| ... and [538 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [017f11f...2825ce0](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] nikolagigic commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
nikolagigic commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r568795817
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -123,6 +123,9 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
adhocMetricLabel: this.state.adhocMetric?.getDefaultLabel(),
});
}
+ if (prevProps.datasource !== this.props.datasource) {
Review comment:
why are we comparing the whole **datasource** object here instead of comparing the **datasource.name** only like here: https://github.com/apache/superset/pull/12782/files#diff-b6ef883f6b46721ba01ae63a0167874a10de1ce512b3fe4cc3535b9df6d19c2eR73 ?
----------------------------------------------------------------
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] [superset] zhaoyongjie merged pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
zhaoyongjie merged pull request #12782:
URL: https://github.com/apache/superset/pull/12782
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (d8c1bf6) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.41%`.
> The diff coverage is `57.95%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.00% -3.42%
==========================================
Files 1022 1022
Lines 50106 50188 +82
Branches 5193 5213 +20
==========================================
- Hits 33283 31622 -1661
- Misses 16680 18351 +1671
- Partials 143 215 +72
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.90% <33.33%> (+0.16%)` | :arrow_up: |
| python | `63.73% <81.48%> (-0.34%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `75.64% <ø> (-10.78%)` | :arrow_down: |
| [...end/src/dashboard/components/dnd/DragDroppable.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9EcmFnRHJvcHBhYmxlLmpzeA==) | `94.59% <ø> (ø)` | |
| [...rc/dashboard/components/dnd/dragDroppableConfig.js](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9kcmFnRHJvcHBhYmxlQ29uZmlnLmpz) | `35.00% <0.00%> (-60.84%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [.../src/explore/components/ControlPanelsContainer.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxzQ29udGFpbmVyLmpzeA==) | `75.51% <0.00%> (-15.61%)` | :arrow_down: |
| [...ore/components/controls/AnnotationLayerControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXJDb250cm9sLmpzeA==) | `8.57% <ø> (-72.86%)` | :arrow_down: |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `19.44% <0.00%> (-1.77%)` | :arrow_down: |
| [...ore/components/controls/DateFilterControl/types.ts](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
| [...ents/controls/FilterControl/AdhocFilterControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyQ29udHJvbC5qc3g=) | `59.09% <ø> (-4.55%)` | :arrow_down: |
| ... and [196 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...dcf9a64](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] ktmud edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769989911
@junlincc Sorry for missing the context. I asked for more information because it was not stated in the roadmap ticket. It would be nice if we can ensure all future tickets have that info.
I know the ticket has been open for a long time, but if I didn't notice this roadmap item, then I doubt others users would have, either.
Again, apologies to both you and @pkdotson for jumping in late, but I think it's worth considering alternatives given how important the control areas are. If you don't think my proposed alternative make sense, then go ahead and implement original proposal. I also don't want @pkdotson 's hard work go wasted.
----------------------------------------------------------------
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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769724776
@junlincc could you elaborate on the motivation behind the original roadmap ticket?
If it is about not showing invalid options, would highlighting them solve the concerns you had?
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (cfbf41a) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.40% -3.03%
==========================================
Files 1022 488 -534
Lines 50106 30173 -19933
Branches 5193 0 -5193
==========================================
- Hits 33283 19130 -14153
+ Misses 16680 11043 -5637
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.40% <ø> (-0.67%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.59% <0.00%> (-3.27%)` | :arrow_down: |
| ... and [538 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...cfbf41a](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (0cbbce2) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `2.73%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.68% -2.74%
==========================================
Files 1022 488 -534
Lines 50106 30145 -19961
Branches 5193 0 -5193
==========================================
- Hits 33283 19199 -14084
+ Misses 16680 10946 -5734
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.68% <ø> (-0.39%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/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/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.22% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.98% <0.00%> (-0.45%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.45% <0.00%> (-0.14%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | | |
| [...frontend/src/components/BootstrapSliderWrapper.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQm9vdHN0cmFwU2xpZGVyV3JhcHBlci5qc3g=) | | |
| ... and [521 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (0cbbce2) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.05%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 63.37% -3.06%
==========================================
Files 1022 488 -534
Lines 50106 30130 -19976
Branches 5193 0 -5193
==========================================
- Hits 33283 19094 -14189
+ Misses 16680 11036 -5644
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.37% <ø> (-0.70%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/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/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `93.40% <0.00%> (-6.05%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `86.55% <0.00%> (-5.47%)` | :arrow_down: |
| ... and [543 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (29a42e0) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `1.37%`.
> The diff coverage is `48.14%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 65.04% -1.38%
==========================================
Files 1022 1026 +4
Lines 50106 48835 -1271
Branches 5193 5257 +64
==========================================
- Hits 33283 31765 -1518
- Misses 16680 16858 +178
- Partials 143 212 +69
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.78% <48.14%> (+0.05%)` | :arrow_up: |
| python | `67.32% <ø> (+3.25%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../src/explore/components/ControlPanelsContainer.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxzQ29udGFpbmVyLmpzeA==) | `75.51% <0.00%> (-15.61%)` | :arrow_down: |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `19.44% <0.00%> (-1.77%)` | :arrow_down: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `68.42% <ø> (-26.32%)` | :arrow_down: |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `70.58% <ø> (-23.53%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `82.55% <ø> (-3.41%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `57.46% <50.00%> (-23.16%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `45.83% <66.66%> (-23.22%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `72.72% <100.00%> (-2.28%)` | :arrow_down: |
| [.../src/explore/components/controls/SelectControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9TZWxlY3RDb250cm9sLmpzeA==) | `87.38% <100.00%> (-6.08%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [235 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...29a42e0](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] ktmud removed a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud removed a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722823
----------------------------------------------------------------
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] [superset] junlincc edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769900048
@ktmud @hushaoqing
This is a request I received from a user forum hosted by organization that has hundreds and thousands Superset daily users. If i remember correctly, you attended that forum as well. This has been an open roadmap item for over couple months so we left more than enough time to collect feedback before implementing the changes.
Given the number of active users in this organization, we assumed that this request is backed by large amount of user research and has reached consensus by most of the users involved. I personally also encounter this pain point in my daily usage of Superset. so the request makes lot of sense to me. and I think @pkdotson did a fabulous job implementing the changes.
Any changes in Superset nowadays can be contentious, that's why we wanna respect and prioritize the requests that is based on some testing results and research large enterprises raised over personal preference or opinion.
We will go ahead with this change and take your suggestion into next iteration.
----------------------------------------------------------------
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] [superset] junlincc commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770122764
> I guess I got a little concerned when you said this is the end of this project:
The end state of this project **for the time being**. We really want to make incremental changes and iterate rapidly instead of holding off the progress because of factors that are out of control.
Back to the solution itself... I am going to respect the original feature request received from the forum for now. let's discuss more next week to explore different options along with other contentious topics. im sure we can reach consensus somehow :) @mihir174 @ktmud
----------------------------------------------------------------
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] [superset] ktmud commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769989911
@junlincc Sorry for missing the context. I asked for more information because it was not stated in the roadmap ticket. It would be nice if we can ensure all future tickets have that info.
I know the ticket has been open for a long time, but if I didn't notice this roadmap item, then I doubt others users would have, either.
Again, apologies to both you and @pkdotson for jumping in late, but I think it's worth considering alternatives given how important the control areas are.
If you don't think my proposed alternative make sense, then go ahead and implement original proposal. I also don't want @pkdotson 's hard work go wasted.
----------------------------------------------------------------
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] [superset] ktmud edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-770119750
I guess I got a little concerned when you said this is the end of this project:
```
end state of this project
- reset/clear/uncheck all query and customize control when dataset is changed
- In the change dataset warning msg, add a line to notify users all the current control settings will be cleared by changing dataset.
```
Rather than resetting everything to default, I think it makes more sense to just reset `SelectControl`, `MetricsControl` and `AdhocFilterControl` to their default state. Because if the end goal is to retain control values for other controls, we already don't have to do anything special about them. The select controls are trickier because you need to check column validity.
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (dcf9a64) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `15.97%`.
> The diff coverage is `59.25%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
===========================================
- Coverage 66.42% 50.44% -15.98%
===========================================
Files 1022 476 -546
Lines 50106 17210 -32896
Branches 5193 4456 -737
===========================================
- Hits 33283 8682 -24601
+ Misses 16680 8528 -8152
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.44% <59.25%> (-0.04%)` | :arrow_down: |
| 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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ore/components/controls/AnnotationLayerControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXJDb250cm9sLmpzeA==) | `81.42% <ø> (ø)` | |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `41.66% <0.00%> (+20.45%)` | :arrow_up: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `88.00% <ø> (-6.74%)` | :arrow_down: |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `88.23% <ø> (-5.89%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `56.72% <ø> (-29.24%)` | :arrow_down: |
| [.../src/explore/components/ControlPanelsContainer.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxzQ29udGFpbmVyLmpzeA==) | `86.73% <37.50%> (-4.38%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `77.44% <50.00%> (-3.18%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `66.66% <66.66%> (-2.39%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `100.00% <100.00%> (+25.00%)` | :arrow_up: |
| [.../src/explore/components/controls/SelectControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9TZWxlY3RDb250cm9sLmpzeA==) | `86.36% <100.00%> (-7.10%)` | :arrow_down: |
| ... and [909 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...dcf9a64](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] junlincc commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769855624
@ktmud this request is from Airbnb superset forums, that you also attended last year. I summarized the request a couple of times and confirm with your team before making the change. We can't afford going back and forth this way.
----------------------------------------------------------------
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] [superset] ktmud edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722823
A better experience is probably to still keep the selected options, but highlight the invalid ones (like Tableau does with invalid calculated columns when you switch a datasource).
----------------------------------------------------------------
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] [superset] villebro commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r567215688
##########
File path: superset-frontend/src/explore/components/ControlPanelsContainer.jsx
##########
@@ -92,6 +92,27 @@ class ControlPanelsContainer extends React.Component {
this.renderControlPanelSection = this.renderControlPanelSection.bind(this);
}
+ componentDidUpdate(prevProps) {
+ const {
+ actions: { setControlValue },
+ } = this.props;
+ if (this.props.form_data.datasource !== prevProps.form_data.datasource) {
+ const defaultValues = [
+ 'MetricsControl',
+ 'AdhocFilterControl',
+ 'TextControl',
+ 'SelectControl',
+ 'CheckboxControl',
+ ];
Review comment:
Probably not a big concern, but `AnnotationLayerControl` would probably be good to include here.
##########
File path: superset-frontend/src/explore/components/ControlPanelsContainer.jsx
##########
@@ -253,7 +274,7 @@ class ControlPanelsContainer extends React.Component {
const expandedCustomSections = this.sectionsToExpand(
displaySectionsToRender,
);
-
+ console.log('controlPanelContainer props: ', this.props);
Review comment:
Oops! 🙂
```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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (29a42e0) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **increase** coverage by `0.78%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
+ Coverage 66.42% 67.20% +0.78%
==========================================
Files 1022 489 -533
Lines 50106 28688 -21418
Branches 5193 0 -5193
==========================================
- Hits 33283 19281 -14002
+ Misses 16680 9407 -7273
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `67.20% <ø> (+3.13%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-17.31%)` | :arrow_down: |
| [superset/db\_engine\_specs/elasticsearch.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2VsYXN0aWNzZWFyY2gucHk=) | `86.95% <0.00%> (-7.49%)` | :arrow_down: |
| [superset/db\_engine\_specs/clickhouse.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2NsaWNraG91c2UucHk=) | `87.09% <0.00%> (-7.35%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `93.42% <0.00%> (-2.64%)` | :arrow_down: |
| ... and [560 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...29a42e0](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] nikolagigic commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
nikolagigic commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r568795817
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -123,6 +123,9 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
adhocMetricLabel: this.state.adhocMetric?.getDefaultLabel(),
});
}
+ if (prevProps.datasource !== this.props.datasource) {
Review comment:
why are we comparing the whole **datasource** object here instead of comparing the **datasource.name** only like here: https://github.com/apache/superset/pull/12782/files#diff-b6ef883f6b46721ba01ae63a0167874a10de1ce512b3fe4cc3535b9df6d19c2eR73 ?
----------------------------------------------------------------
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] [superset] junlincc commented on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769900048
@ktmud @hushaoqing
This is a request I received from a user forum hosted by organization that has hundreds and thousands Superset daily users. If i remember correctly, you attended that forum as well. This has been an open roadmap item for over couple months so we left more than enough time to collect feedback before implementing the changes.
Given the number of active users in this organization, we assumed that this request is backed by large amount of user research and has reached consensus by most of the users involved. I personally also encounter this pain point in my daily usage of Superset. so the request makes lot of sense to me. and I think @pkdotson did a fabulous job implementing the changes.
Any changes in Superset nowadays can be contentious, that's why we wanna respect and prioritize the requests from large enterprises raised, based on some testing results and research over personal preference or opinion.
We will go ahead with this change.
----------------------------------------------------------------
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] [superset] pkdotson commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
pkdotson commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r569011801
##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -123,6 +123,9 @@ export default class AdhocMetricEditPopover extends React.PureComponent {
adhocMetricLabel: this.state.adhocMetric?.getDefaultLabel(),
});
}
+ if (prevProps.datasource !== this.props.datasource) {
Review comment:
This does refer's to the the datasource name. datasource is datasource name being passed down
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: [wip] reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (2825ce0) into [master](https://codecov.io/gh/apache/superset/commit/017f11f9d84ac88019a6c42eb65a67b617966e95?el=desc) (017f11f) will **increase** coverage by `0.65%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
+ Coverage 63.13% 63.79% +0.65%
==========================================
Files 1022 488 -534
Lines 50032 30117 -19915
Branches 4915 0 -4915
==========================================
- Hits 31587 19212 -12375
+ Misses 18245 10905 -7340
+ Partials 200 0 -200
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `?` | |
| python | `63.79% <ø> (-0.31%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.86% <0.00%> (-2.99%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `72.93% <0.00%> (-2.46%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/datasets/api.py](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YXNldHMvYXBpLnB5) | `89.49% <0.00%> (-1.83%)` | :arrow_down: |
| ... and [535 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [017f11f...2825ce0](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] junlincc removed a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc removed a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769855624
@ktmud this request is from Airbnb superset forums, that you also attended last year. I summarized the request a couple of times and confirm with your team before making the change. We can't afford going back and forth this way.
----------------------------------------------------------------
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] [superset] ktmud edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722211
I think it would be nice to retain metrics/columns with matching column names at least since a lot of times users would want to fix a broken chart or iterate on an existing one by replacing the datasource with a newer (and slightly updated) dataset.
I know this requirement exists because as a former Data Scientist, I've done similar operation in Tableau a lot.
----------------------------------------------------------------
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] [superset] zhaoyongjie commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r569983349
##########
File path: superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx
##########
@@ -157,7 +157,6 @@ class AnnotationLayerControl extends React.PureComponent {
render() {
const { addedAnnotationIndex } = this.state;
const addedAnnotation = this.props.value[addedAnnotationIndex];
-
Review comment:
This file has not been modified. Could we avoid remove this line?
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (0cbbce2) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `3.50%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
==========================================
- Coverage 66.42% 62.92% -3.51%
==========================================
Files 1022 1022
Lines 50106 50112 +6
Branches 5193 5206 +13
==========================================
- Hits 33283 31531 -1752
- Misses 16680 18370 +1690
- Partials 143 211 +68
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.70% <42.85%> (-0.04%)` | :arrow_down: |
| python | `63.72% <ø> (-0.35%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ore/components/controls/AnnotationLayerControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXJDb250cm9sLmpzeA==) | `8.33% <0.00%> (-73.10%)` | :arrow_down: |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `19.44% <0.00%> (-1.77%)` | :arrow_down: |
| [...ents/controls/FilterControl/AdhocFilterControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyQ29udHJvbC5qc3g=) | `58.20% <0.00%> (-5.43%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `75.00% <ø> (ø)` | |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `68.42% <ø> (-26.32%)` | :arrow_down: |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `70.58% <ø> (-23.53%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `39.58% <16.66%> (-29.47%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `58.77% <50.00%> (-21.85%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `82.65% <50.00%> (-3.31%)` | :arrow_down: |
| [...rc/explore/components/controls/CheckboxControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9DaGVja2JveENvbnRyb2wuanN4) | `82.35% <66.66%> (-3.37%)` | :arrow_down: |
| ... and [161 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...0cbbce2](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] ktmud edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769722211
I think it would be nice to retain metrics/columns with matching column names at least since a lot of times users would want to fix a broken chart or iterate on an existing one by replacing the datasource with a newer (and slightly updated) dataset.
I know this requirement exists because as a former Data Scientist, I've done similar operation in Tableau a lot.
A better experience is probably to still keep the selected options, but highlight the invalid ones (like Tableau does with invalid calculated columns when you switch a datasource).
@junlincc could you elaborate on the motivation behind the original roadmap ticket? If it is about not showing invalid options, would highlighting them solve the concerns you had?
----------------------------------------------------------------
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] [superset] junlincc edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-769900048
@ktmud @hushaoqing
This is a request I received from a user forum hosted by organization that has hundreds and thousands Superset daily users. If i remember correctly, you attended that forum as well. This has been an open roadmap item for over couple months so we left more than enough time to collect feedback before implementing the changes.
Given the number of active users in this organization, we assumed that this request is backed by large amount of user research and has reached consensus by most of the users involved. I personally also encounter this pain point in my daily usage of Superset. so the request makes lot of sense to me. and I think @pkdotson did a fabulous job implementing the changes.
Any changes in Superset nowadays can be contentious, that's why we wanna respect and prioritize the requests that is based on some testing results and research large enterprises raised over personal preference or opinion.
We will go ahead with this change.
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
# [Codecov](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=h1) Report
> Merging [#12782](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=desc) (dcf9a64) into [master](https://codecov.io/gh/apache/superset/commit/fa8c492acd91ac31483134d5abf0d35ec2c0dfd2?el=desc) (fa8c492) will **decrease** coverage by `15.60%`.
> The diff coverage is `59.25%`.
[](https://codecov.io/gh/apache/superset/pull/12782?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12782 +/- ##
===========================================
- Coverage 66.42% 50.81% -15.61%
===========================================
Files 1022 476 -546
Lines 50106 17210 -32896
Branches 5193 4456 -737
===========================================
- Hits 33283 8746 -24537
+ Misses 16680 8464 -8216
+ Partials 143 0 -143
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.81% <59.25%> (+0.33%)` | :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/superset/pull/12782?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ore/components/controls/AnnotationLayerControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Bbm5vdGF0aW9uTGF5ZXJDb250cm9sLmpzeA==) | `81.42% <ø> (ø)` | |
| [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | `41.66% <0.00%> (+20.45%)` | :arrow_up: |
| [...ntrols/MetricControl/AdhocMetricPopoverTrigger.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljUG9wb3ZlclRyaWdnZXIudHN4) | `88.00% <ø> (-6.74%)` | :arrow_down: |
| [...s/controls/MetricControl/MetricDefinitionValue.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY0RlZmluaXRpb25WYWx1ZS5qc3g=) | `88.23% <ø> (-5.89%)` | :arrow_down: |
| [...mponents/controls/MetricControl/MetricsControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL01ldHJpY3NDb250cm9sLmpzeA==) | `56.72% <ø> (-29.24%)` | :arrow_down: |
| [.../src/explore/components/ControlPanelsContainer.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sUGFuZWxzQ29udGFpbmVyLmpzeA==) | `86.73% <37.50%> (-4.38%)` | :arrow_down: |
| [.../controls/MetricControl/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljRWRpdFBvcG92ZXIuanN4) | `77.44% <50.00%> (-3.18%)` | :arrow_down: |
| [...nd/src/explore/components/controls/TextControl.tsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UZXh0Q29udHJvbC50c3g=) | `66.66% <66.66%> (-2.39%)` | :arrow_down: |
| [...nents/controls/MetricControl/AdhocMetricOption.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9NZXRyaWNDb250cm9sL0FkaG9jTWV0cmljT3B0aW9uLmpzeA==) | `100.00% <100.00%> (+25.00%)` | :arrow_up: |
| [.../src/explore/components/controls/SelectControl.jsx](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9TZWxlY3RDb250cm9sLmpzeA==) | `86.36% <100.00%> (-7.10%)` | :arrow_down: |
| ... and [909 more](https://codecov.io/gh/apache/superset/pull/12782/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12782?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/superset/pull/12782?src=pr&el=footer). Last update [fa8c492...dcf9a64](https://codecov.io/gh/apache/superset/pull/12782?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] [superset] zhaoyongjie commented on a change in pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #12782:
URL: https://github.com/apache/superset/pull/12782#discussion_r569983349
##########
File path: superset-frontend/src/explore/components/controls/AnnotationLayerControl.jsx
##########
@@ -157,7 +157,6 @@ class AnnotationLayerControl extends React.PureComponent {
render() {
const { addedAnnotationIndex } = this.state;
const addedAnnotation = this.props.value[addedAnnotationIndex];
-
Review comment:
This file has not been modified. Could we avoid remove this line?
----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12782: feat: reset metrics on dataset change
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12782:
URL: https://github.com/apache/superset/pull/12782#issuecomment-767930577
----------------------------------------------------------------
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