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/08/03 19:56:46 UTC

[GitHub] [superset] ktmud opened a new pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

ktmud opened a new pull request #16053:
URL: https://github.com/apache/superset/pull/16053


   ### SUMMARY
   
   Fix a 500 error on the `/dashboard/<dashboardId>/datasets` API caused by an incorrect nested list comprehension introduced by https://github.com/apache/superset/pull/15993 .
   
   Updated the test case to cover this case.
   
   In dashboards where a slice has the `y` metric set (e.g. a NVD3 timeseries chart), the `/datasets` API will return 500. However, most of the time if will not break the dashboard, since this API is almost only used in FilterBox and filter indicators.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A
   
   ### TESTING INSTRUCTIONS
   
   1. Test adding a NVD3 Timeseries chart to the dashboard
   2. You may see 500 error in base but not error after the fix
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: 
   - [ ] 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] ktmud commented on a change in pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16053:
URL: https://github.com/apache/superset/pull/16053#discussion_r682061228



##########
File path: superset/utils/core.py
##########
@@ -1464,12 +1463,7 @@ def get_column_names_from_metrics(metrics: List[Metric]) -> List[str]:
     :param metrics: Ad-hoc metric
     :return: column name if simple metric, otherwise None
     """
-    columns: List[str] = []
-    for metric in metrics:
-        column_name = get_column_name_from_metric(metric)
-        if column_name:
-            columns.append(column_name)
-    return columns
+    return [col for col in map(get_column_name_from_metric, metrics) if col]

Review comment:
       Bycatch refactor. No real code logic changes.




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

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 #16053: fix(dashboard): 500 error caused by data_for_slices API

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


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16053?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 [#16053](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a7e5eef) into [master](https://codecov.io/gh/apache/superset/commit/69c5cd792296167d503403455b7e434fb3fedcd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69c5cd7) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head a7e5eef differs from pull request most recent head fd3b68b. Consider uploading reports for the commit fd3b68b to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16053/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/16053?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   #16053      +/-   ##
   ==========================================
   - Coverage   76.66%   76.66%   -0.01%     
   ==========================================
     Files         995      995              
     Lines       52869    52864       -5     
     Branches     6712     6712              
   ==========================================
   - Hits        40533    40528       -5     
     Misses      12110    12110              
     Partials      226      226              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | mysql | `81.59% <100.00%> (-0.01%)` | :arrow_down: |
   | postgres | `81.58% <100.00%> (-0.01%)` | :arrow_down: |
   | python | `81.71% <100.00%> (-0.01%)` | :arrow_down: |
   | sqlite | `81.22% <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/16053?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/viz.py](https://codecov.io/gh/apache/superset/pull/16053/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) | `56.57% <ø> (ø)` | |
   | [superset/connectors/base/models.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `88.06% <100.00%> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `88.09% <100.00%> (-0.08%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16053?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/16053?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 [69c5cd7...fd3b68b](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

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

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



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

[GitHub] [superset] codecov[bot] edited a comment on pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

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


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16053?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 [#16053](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3839452) into [master](https://codecov.io/gh/apache/superset/commit/69c5cd792296167d503403455b7e434fb3fedcd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69c5cd7) will **decrease** coverage by `0.14%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 3839452 differs from pull request most recent head f0fd52c. Consider uploading reports for the commit f0fd52c to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16053/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/16053?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   #16053      +/-   ##
   ==========================================
   - Coverage   76.81%   76.66%   -0.15%     
   ==========================================
     Files         995      995              
     Lines       52869    52864       -5     
     Branches     6712     6712              
   ==========================================
   - Hits        40610    40528      -82     
   - Misses      12033    12110      +77     
     Partials      226      226              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `81.59% <100.00%> (-0.01%)` | :arrow_down: |
   | postgres | `81.62% <100.00%> (+0.03%)` | :arrow_up: |
   | python | `81.71% <100.00%> (-0.29%)` | :arrow_down: |
   | sqlite | `81.22% <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/16053?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/viz.py](https://codecov.io/gh/apache/superset/pull/16053/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) | `56.57% <ø> (ø)` | |
   | [superset/connectors/base/models.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `88.06% <100.00%> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `88.09% <100.00%> (-0.21%)` | :arrow_down: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-82.15%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.80% <0.00%> (-16.87%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `83.47% <0.00%> (-1.05%)` | :arrow_down: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `87.98% <0.00%> (-0.39%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `88.08% <0.00%> (-0.24%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16053?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/16053?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 [69c5cd7...f0fd52c](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

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

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



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

[GitHub] [superset] codecov[bot] edited a comment on pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

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


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16053?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 [#16053](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3839452) into [master](https://codecov.io/gh/apache/superset/commit/69c5cd792296167d503403455b7e434fb3fedcd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69c5cd7) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 3839452 differs from pull request most recent head fd3b68b. Consider uploading reports for the commit fd3b68b to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16053/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/16053?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   #16053      +/-   ##
   ==========================================
   - Coverage   76.66%   76.64%   -0.02%     
   ==========================================
     Files         995      995              
     Lines       52869    52823      -46     
     Branches     6712     6712              
   ==========================================
   - Hits        40533    40488      -45     
   + Misses      12110    12109       -1     
     Partials      226      226              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | mysql | `81.59% <100.00%> (-0.01%)` | :arrow_down: |
   | postgres | `81.59% <100.00%> (+0.01%)` | :arrow_up: |
   | python | `81.68% <100.00%> (-0.03%)` | :arrow_down: |
   | sqlite | `81.22% <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/16053?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/viz.py](https://codecov.io/gh/apache/superset/pull/16053/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) | `56.57% <ø> (ø)` | |
   | [superset/connectors/base/models.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `88.06% <100.00%> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `88.09% <100.00%> (-0.08%)` | :arrow_down: |
   | [superset/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `74.77% <0.00%> (-2.22%)` | :arrow_down: |
   | [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `92.59% <0.00%> (-0.27%)` | :arrow_down: |
   | [superset/utils/cache.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvdXRpbHMvY2FjaGUucHk=) | `74.28% <0.00%> (-0.25%)` | :arrow_down: |
   | [superset/common/query\_context.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2NvbnRleHQucHk=) | `90.52% <0.00%> (-0.22%)` | :arrow_down: |
   | [superset/reports/notifications/base.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2Jhc2UucHk=) | `95.65% <0.00%> (-0.19%)` | :arrow_down: |
   | [superset/utils/log.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvdXRpbHMvbG9nLnB5) | `92.95% <0.00%> (-0.15%)` | :arrow_down: |
   | [superset/migrations/shared/security\_converge.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvbWlncmF0aW9ucy9zaGFyZWQvc2VjdXJpdHlfY29udmVyZ2UucHk=) | `86.82% <0.00%> (-0.11%)` | :arrow_down: |
   | ... and [6 more](https://codecov.io/gh/apache/superset/pull/16053/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/16053?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/16053?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 [69c5cd7...fd3b68b](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

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

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



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

[GitHub] [superset] codecov[bot] edited a comment on pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

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


   # [Codecov](https://codecov.io/gh/apache/superset/pull/16053?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 [#16053](https://codecov.io/gh/apache/superset/pull/16053?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3839452) into [master](https://codecov.io/gh/apache/superset/commit/69c5cd792296167d503403455b7e434fb3fedcd6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (69c5cd7) will **decrease** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 3839452 differs from pull request most recent head fd3b68b. Consider uploading reports for the commit fd3b68b to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/16053/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/16053?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   #16053      +/-   ##
   ==========================================
   - Coverage   76.66%   76.66%   -0.01%     
   ==========================================
     Files         995      995              
     Lines       52869    52864       -5     
     Branches     6712     6712              
   ==========================================
   - Hits        40533    40528       -5     
     Misses      12110    12110              
     Partials      226      226              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | mysql | `81.59% <100.00%> (-0.01%)` | :arrow_down: |
   | postgres | `81.62% <100.00%> (+0.03%)` | :arrow_up: |
   | python | `81.71% <100.00%> (-0.01%)` | :arrow_down: |
   | sqlite | `81.22% <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/16053?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/viz.py](https://codecov.io/gh/apache/superset/pull/16053/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) | `56.57% <ø> (ø)` | |
   | [superset/connectors/base/models.py](https://codecov.io/gh/apache/superset/pull/16053/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-c3VwZXJzZXQvY29ubmVjdG9ycy9iYXNlL21vZGVscy5weQ==) | `88.06% <100.00%> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/16053/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==) | `88.09% <100.00%> (-0.08%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16053?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/16053?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 [69c5cd7...fd3b68b](https://codecov.io/gh/apache/superset/pull/16053?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] ktmud commented on a change in pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #16053:
URL: https://github.com/apache/superset/pull/16053#discussion_r682061016



##########
File path: superset/connectors/base/models.py
##########
@@ -308,8 +307,8 @@ def data_for_slices(self, slices: List[Slice]) -> Dict[str, Any]:
 
             column_names.update(
                 column
-                for column in utils.get_iterable(form_data.get(param) or [])
-                for param in COLUMN_FORM_DATA_PARAMS
+                for column_param in COLUMN_FORM_DATA_PARAMS
+                for column in utils.get_iterable(form_data.get(column_param) or [])

Review comment:
       The order of the for loop is incorrect. Normally it will throw an undefined variable error but since `param` was used in other loops above, the code will still run. Just another example of why too many local variables is not a good thing.




-- 
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] ktmud merged pull request #16053: fix(dashboard): 500 error caused by data_for_slices API

Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #16053:
URL: https://github.com/apache/superset/pull/16053


   


-- 
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