[migration]clean up __from and __to in dashboard json_metadata #6378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
from
424dcbb
to
2c70db4
Compare
Codecov Report
@@ Coverage Diff @@
## master #6378 +/- ##
=======================================
Coverage 77.31% 77.31%
=======================================
Files 67 67
Lines 9581 9581
=======================================
Hits 7408 7408
Misses 2173 2173Continue to review full report at Codecov.
|
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
from
2c70db4
to
b0a1123
Compare
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
2 times, most recently
from
b020596
to
476b2ce
Compare
john-bodley
reviewed
Nov 15, 2018
|
|
||
| if has_update: | ||
| dashboard.json_metadata = json.dumps(json_metadata) | ||
| session.merge(dashboard) |
There was a problem hiding this comment.
We don't need to do a merge and having a single commit at the end is more efficient.
| for key in keys: | ||
| val = filters[key] | ||
| val['__time_range'] = '{} : {}'.format( | ||
| val.pop('__from', '') or '', |
There was a problem hiding this comment.
I don't think we need the or.
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
from
476b2ce
to
cdc0658
Compare
|
code is updated per comment. also tested in dev environment...much faster!! |
john-bodley
reviewed
Nov 15, 2018
| val.pop('__from', ''), | ||
| val.pop('__to', ''), | ||
| ) | ||
| json_metadata.update( |
There was a problem hiding this comment.
Nit. json_metadata['default_filters'] = json.dumps(filters).
| if '__to' in val: | ||
| val.remove('__to') | ||
| val.append('__time_range') | ||
| json_metadata.update( |
There was a problem hiding this comment.
Nit. json_metadata['filter_immune_slice_fields'] = filter_immune_slice_fields .
| if len(keys): | ||
| for key in keys: | ||
| val = filters[key] | ||
| val['__time_range'] = '{} : {}'.format( |
There was a problem hiding this comment.
Is there any concern that __time_range could already exist? If so should it trump the __from and __to?
There was a problem hiding this comment.
yes, i think should check if __time_range exist. if it is exist, just abandon __from and __to parameter, so that not override existed __time_range.
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
from
cdc0658
to
dec824f
Compare
graceguo-supercat
force-pushed
the
gg-MigrateDashboardMetadata
branch
from
dec824f
to
5219596
Compare
john-bodley
approved these changes
Nov 16, 2018
graceguo-supercat
pushed a commit
to graceguo-supercat/superset
that referenced
this pull request
Nov 16, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
michellethomas
pushed a commit
to michellethomas/panoramix
that referenced
this pull request
Nov 20, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 18, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 18, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 19, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
bipinsoniguavus
pushed a commit
to ThalesGroup/incubator-superset
that referenced
this pull request
Dec 26, 2018
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 26, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Dec 28, 2018
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 4, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 9, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
xtinec
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 9, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 11, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 12, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
youngyjd
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 14, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 17, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 24, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 30, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
betodealmeida
pushed a commit
to lyft/incubator-superset
that referenced
this pull request
Jan 30, 2019
…data (apache#6378) (cherry picked from commit c42bcf8)
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In dashboard Superset allows user save default values for filter slice, and saved in json_metadata. So that when the dashboard is loaded, it will be pre-populated with values for the filters. Superset also allows user set charts that immune from filters. For example:
{"filter_immune_slices": [], "timed_refresh_immune_slices": [], "filter_immune_slice_fields": {"255": ["__from", "__to"]}, "expanded_slices": {}, "default_filters": "{\"254\": {\"__from\": \"1 months ago\", \"dim_country\": [\"all\"]}}"}after recently introduced time_range, __from and __to in json_metadata didn't correctly merge with time_range. It introduces extra __from/__to parameter into Filter control, and cause chart show
no data. In airbnb we have about 500 dashboards set __from and __to in json_metadata.Fix: I wrote this migration script to update __from/__to to __time_range for json_metadata.
@betodealmeida @mistercrunch @john-bodley @timifasubaa @michellethomas