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/10/26 18:58:12 UTC

[GitHub] [superset] john-bodley opened a new pull request #17236: fix: Revert default series sort-by metric and enforce non-xor with series limit

john-bodley opened a new pull request #17236:
URL: https://github.com/apache/superset/pull/17236


   ### SUMMARY
   
   This PR is one of a slew to the [apache/superset](https://github.com/apache/superset) and [apache-superset/superset-ui](https://github.com/apache-superset/superset-ui) repos to revert pseudo recent changes to the series restrictions for high cardinality groupings. For more context please refer to [this](https://apache-superset.slack.com/archives/G013HAE6Y0K/p1635192289021500) Slack thread in the #committers channel.
   
   Specifically this PR reverts https://github.com/apache/superset/pull/15343, i.e., does not fallback to the first selected metric and requires the user to specify a sort-by metric for improved UX. Note previously there was inconsistency in the results when no ordering was specified and thus this PR ensures that a series limit must be defined in conjunction with a series sort-by metric. Ideally the UI controls would handle this logic, however for now it merely validates the query object.
   
   Finally this PR re-orders some of the UI elements, i.e., the series limit metric is defined prior to the limit as this is more inline with how people think about querying, i.e., one normally specifies the `ORDER BY` clause prior to the `LIMIT` clause.
   
   Related PRs:
   
   - https://github.com/apache-superset/superset-ui/pull/1430
   - https://github.com/apache-superset/superset-ui/pull/1436
   
   ### TESTING INSTRUCTIONS
   
   Added unit tests and tested manually.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] 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.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley commented on pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-956490569


   @zhaoyongjie any thoughts/comments regarding this in light of the comments I added [here](https://apache-superset.slack.com/archives/G013HAE6Y0K/p1635192289021500). 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley edited a comment on pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-956490569


   @zhaoyongjie any thoughts/comments regarding this in light of the comments I added [here](https://apache-superset.slack.com/archives/G013HAE6Y0K/p1635192289021500)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley merged pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #17236:
URL: https://github.com/apache/superset/pull/17236


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17236: fix: revert default series sort-by metric and enforce non-xor with series limit

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-952361673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17236](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86a6612) into [master](https://codecov.io/gh/apache/superset/commit/dd71035e58fa466b9f1889ca32c2c71c39387c88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dd71035) will **increase** coverage by `0.00%`.
   > The diff coverage is `93.75%`.
   
   > :exclamation: Current head 86a6612 differs from pull request most recent head 8fbdc32. Consider uploading reports for the commit 8fbdc32 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17236/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #17236   +/-   ##
   =======================================
     Coverage   76.95%   76.95%           
   =======================================
     Files        1038     1037    -1     
     Lines       55612    55609    -3     
     Branches     7590     7588    -2     
   =======================================
   - Hits        42798    42796    -2     
   + Misses      12564    12563    -1     
     Partials      250      250           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `71.01% <100.00%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.14% <ø> (ø)` | |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `87.48% <92.85%> (ø)` | |
   | [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `29.72% <100.00%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `57.99% <100.00%> (ø)` | |
   | [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `87.13% <0.00%> (-0.59%)` | :arrow_down: |
   | [...perset-frontend/src/components/ChartIcon/index.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnRJY29uL2luZGV4LnRzeA==) | | |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `90.37% <0.00%> (+0.41%)` | :arrow_up: |
   | ... and [1 more](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dd71035...8fbdc32](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #17236: fix: revert default series sort-by metric and enforce non-xor with series limit

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-952361673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17236](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5d8a45) into [master](https://codecov.io/gh/apache/superset/commit/dd71035e58fa466b9f1889ca32c2c71c39387c88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dd71035) will **decrease** coverage by `0.00%`.
   > The diff coverage is `93.33%`.
   
   > :exclamation: Current head f5d8a45 differs from pull request most recent head 9652004. Consider uploading reports for the commit 9652004 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17236/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17236      +/-   ##
   ==========================================
   - Coverage   76.95%   76.95%   -0.01%     
   ==========================================
     Files        1038     1037       -1     
     Lines       55612    55609       -3     
     Branches     7590     7588       -2     
   ==========================================
   - Hits        42798    42795       -3     
     Misses      12564    12564              
     Partials      250      250              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `71.01% <ø> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `29.72% <ø> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.14% <ø> (ø)` | |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `87.48% <92.85%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `57.99% <100.00%> (ø)` | |
   | [...perset-frontend/src/components/ChartIcon/index.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnRJY29uL2luZGV4LnRzeA==) | | |
   | [...ponents/filterscope/renderFilterScopeTreeNodes.jsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2ZpbHRlcnNjb3BlL3JlbmRlckZpbHRlclNjb3BlVHJlZU5vZGVzLmpzeA==) | `87.50% <0.00%> (+0.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dd71035...9652004](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley commented on pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley commented on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-956490569


   @zhaoyongjie any thoughts/comments regarding this in light of the comments I added [here](https://apache-superset.slack.com/archives/G013HAE6Y0K/p1635192289021500). 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17236: fix: revert default series sort-by metric and enforce non-xor with series limit

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-952361673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17236](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86a6612) into [master](https://codecov.io/gh/apache/superset/commit/dd71035e58fa466b9f1889ca32c2c71c39387c88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dd71035) will **increase** coverage by `0.00%`.
   > The diff coverage is `93.75%`.
   
   > :exclamation: Current head 86a6612 differs from pull request most recent head 9652004. Consider uploading reports for the commit 9652004 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17236/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #17236   +/-   ##
   =======================================
     Coverage   76.95%   76.95%           
   =======================================
     Files        1038     1037    -1     
     Lines       55612    55609    -3     
     Branches     7590     7588    -2     
   =======================================
   - Hits        42798    42796    -2     
   + Misses      12564    12563    -1     
     Partials      250      250           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `71.01% <100.00%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.14% <ø> (ø)` | |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `87.48% <92.85%> (ø)` | |
   | [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `29.72% <100.00%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `57.99% <100.00%> (ø)` | |
   | [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `87.13% <0.00%> (-0.59%)` | :arrow_down: |
   | [...perset-frontend/src/components/ChartIcon/index.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnRJY29uL2luZGV4LnRzeA==) | | |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `90.37% <0.00%> (+0.41%)` | :arrow_up: |
   | ... and [1 more](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dd71035...9652004](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley merged pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #17236:
URL: https://github.com/apache/superset/pull/17236


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17236: fix: revert default series sort-by metric and enforce non-xor with series limit

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-952361673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17236](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86a6612) into [master](https://codecov.io/gh/apache/superset/commit/dd71035e58fa466b9f1889ca32c2c71c39387c88?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dd71035) will **increase** coverage by `0.00%`.
   > The diff coverage is `93.75%`.
   
   > :exclamation: Current head 86a6612 differs from pull request most recent head 8f659aa. Consider uploading reports for the commit 8f659aa to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17236/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #17236   +/-   ##
   =======================================
     Coverage   76.95%   76.95%           
   =======================================
     Files        1038     1037    -1     
     Lines       55612    55609    -3     
     Branches     7590     7588    -2     
   =======================================
   - Hits        42798    42796    -2     
   + Misses      12564    12563    -1     
     Partials      250      250           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `71.01% <100.00%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `90.14% <ø> (ø)` | |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `87.48% <92.85%> (ø)` | |
   | [superset-frontend/src/explore/controls.jsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29udHJvbHMuanN4) | `29.72% <100.00%> (ø)` | |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `57.99% <100.00%> (ø)` | |
   | [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `87.13% <0.00%> (-0.59%)` | :arrow_down: |
   | [...perset-frontend/src/components/ChartIcon/index.tsx](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2hhcnRJY29uL2luZGV4LnRzeA==) | | |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `90.37% <0.00%> (+0.41%)` | :arrow_up: |
   | ... and [1 more](https://codecov.io/gh/apache/superset/pull/17236/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dd71035...8f659aa](https://codecov.io/gh/apache/superset/pull/17236?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley commented on a change in pull request #17236: fix: revert default series sort-by metric and enforce non-xor with series limit

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17236:
URL: https://github.com/apache/superset/pull/17236#discussion_r736939490



##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -92,7 +92,7 @@ export const D3_FORMAT_OPTIONS = [
 
 const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];
 
-const SERIES_LIMITS = [0, 5, 10, 25, 50, 100, 500];
+const SERIES_LIMITS = [5, 10, 25, 50, 100, 500];

Review comment:
       Consistent with https://github.com/apache-superset/superset-ui/pull/1430. Note I'm unsure why things are defined in multiple places.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley edited a comment on pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on pull request #17236:
URL: https://github.com/apache/superset/pull/17236#issuecomment-956490569


   @zhaoyongjie any thoughts/comments regarding this in light of the comments I added [here](https://apache-superset.slack.com/archives/G013HAE6Y0K/p1635192289021500)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley merged pull request #17236: fix: revert default series sort-by metric

Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #17236:
URL: https://github.com/apache/superset/pull/17236






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org