You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/02/14 22:03:31 UTC
[GitHub] [incubator-superset] graceguo-supercat opened a new pull request
#9146: [dashboard] clean up usage for old filter immune metadata
graceguo-supercat opened a new pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146
### CATEGORY
Choose one
- [x] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
After migration/convert old filter immune attribute in #9109 and #9145, dashboard front-end doesn't have to handle backward compatibility issue with old metadata. This PR is to clean up the usage for `filter_immune_slices` and `filter_immune_slice_fields` everywhere.
### TEST PLAN
CI and manual tests.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: #9109 #9145
### REVIEWERS
@john-bodley @serenajiang @etr2460 @mistercrunch
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in
pull request #9146: [dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146#discussion_r380945332
##########
File path: superset/views/core.py
##########
@@ -1263,16 +1263,8 @@ def _set_dash_metadata(dashboard, data):
if "timed_refresh_immune_slices" not in md:
md["timed_refresh_immune_slices"] = []
-
if "filter_scopes" in data:
- md.pop("filter_immune_slices", None)
- md.pop("filter_immune_slice_fields", None)
md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}"))
Review comment:
there might be many dashboards holding NULL value in `data.get('filter_scopes')`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat merged pull request #9146:
[dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on a change in
pull request #9146: [dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146#discussion_r381032592
##########
File path: superset/views/core.py
##########
@@ -1263,16 +1263,8 @@ def _set_dash_metadata(dashboard, data):
if "timed_refresh_immune_slices" not in md:
md["timed_refresh_immune_slices"] = []
-
if "filter_scopes" in data:
- md.pop("filter_immune_slices", None)
- md.pop("filter_immune_slice_fields", None)
md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}"))
Review comment:
you are right! added fix. thank you!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9146:
[dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146#issuecomment-586503457
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=h1) Report
> Merging [#9146](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/f95a8677423eb5515c13d466a395329e158ab9b6?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9146 +/- ##
=========================================
+ Coverage 59.06% 59.1% +0.03%
=========================================
Files 372 372
Lines 11922 11915 -7
Branches 2919 2917 -2
=========================================
Hits 7042 7042
+ Misses 4698 4693 -5
+ Partials 182 180 -2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/reducers/getInitialState.js](https://codecov.io/gh/apache/incubator-superset/pull/9146/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `0% <ø> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9146?src=pr&el=footer). Last update [f95a867...a7d0070](https://codecov.io/gh/apache/incubator-superset/pull/9146?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] serenajiang commented on a change in pull
request #9146: [dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
serenajiang commented on a change in pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146#discussion_r380882694
##########
File path: superset/views/core.py
##########
@@ -1263,16 +1263,8 @@ def _set_dash_metadata(dashboard, data):
if "timed_refresh_immune_slices" not in md:
md["timed_refresh_immune_slices"] = []
-
if "filter_scopes" in data:
- md.pop("filter_immune_slices", None)
- md.pop("filter_immune_slice_fields", None)
md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}"))
Review comment:
```suggestion
md["filter_scopes"] = json.loads(data["filter_scopes"])
```
not yours, but might as well clean this up
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] serenajiang commented on a change in pull
request #9146: [dashboard] clean up usage for old filter immune metadata
Posted by GitBox <gi...@apache.org>.
serenajiang commented on a change in pull request #9146: [dashboard] clean up usage for old filter immune metadata
URL: https://github.com/apache/incubator-superset/pull/9146#discussion_r381009769
##########
File path: superset/views/core.py
##########
@@ -1263,16 +1263,8 @@ def _set_dash_metadata(dashboard, data):
if "timed_refresh_immune_slices" not in md:
md["timed_refresh_immune_slices"] = []
-
if "filter_scopes" in data:
- md.pop("filter_immune_slices", None)
- md.pop("filter_immune_slice_fields", None)
md["filter_scopes"] = json.loads(data.get("filter_scopes", "{}"))
Review comment:
Since, this is inside the if statement `if "filter_scopes" in data:`, we don't need to use `.get` because we know the key is there. For the case you mentioned, I think this should be:
```python
md["filter_scopes"] = json.loads(data["filter_scopes"] or "{}")
```
For explanation,
```python
data = {"filter_scopes": None}
print(data.get("filter_scopes", "{}")) # prints None
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org