chore: Cleaning up ENABLE_REACT_CRUD_VIEWS config #11496
Merged
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
nytai
approved these changes
Oct 29, 2020
Comment on lines
-852
to
-854
| # Enables the replacement react views for all the FAB views (list, edit, show) with | ||
| # designs introduced in SIP-34: https://github.com/apache/incubator-superset/issues/8976 | ||
| # This is a work in progress so not all features available in FAB have been implemented |
There was a problem hiding this comment.
Should we move these comments to the feature flag lines?
john-bodley
force-pushed
the
john-bodley--cleanup-enable-react-crud-views-feature-flag
branch
2 times, most recently
from
30c3edb
to
39e076d
Compare
Codecov Report
@@ Coverage Diff @@
## master #11496 +/- ##
===========================================
- Coverage 66.64% 56.43% -10.21%
===========================================
Files 863 407 -456
Lines 41230 13741 -27489
Branches 3719 3502 -217
===========================================
- Hits 27477 7755 -19722
+ Misses 13653 5819 -7834
- Partials 100 167 +67
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
The CI is breaking probably because of this: https://github.com/apache/incubator-superset/blob/39e076d0566b12c115c6098fe5adaadec9cb7176/tests/superset_test_config.py#L76 I think we should clean this up and update def is_feature_enabled(self, feature: str) -> bool:
"""Utility function for checking whether a feature is turned on"""
env_key = f"SUPERSET_{feature}"
if env_key in os.env:
return True
feature_flags = self.get_feature_flags()
if feature_flags and feature in feature_flags:
return feature_flags[feature]
return False |
john-bodley
force-pushed
the
john-bodley--cleanup-enable-react-crud-views-feature-flag
branch
from
39e076d
to
8c476b3
Compare
john-bodley
commented
Oct 29, 2020
| @@ -55,6 +55,7 @@ | |||
| "KV_STORE": True, | |||
| "SHARE_QUERIES_VIA_KV_STORE": True, | |||
| "ENABLE_TEMPLATE_PROCESSING": True, | |||
| "ENABLE_REACT_CRUD_VIEWS": os.environ.get("ENABLE_REACT_CRUD_VIEWS", False), | |||
There was a problem hiding this comment.
@ktmud this should hopefully resolve the issue.
ktmud
approved these changes
Oct 29, 2020
john-bodley
deleted the
john-bodley--cleanup-enable-react-crud-views-feature-flag
branch
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
Co-authored-by: John Bodley <john.bodley@airbnb.com>
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.
SUMMARY
Now that
ENABLE_REACT_CRUD_VIEWSis behind a feature flag (#11371) there seems to be merit in deprecating the config parameter with the same name (DRY) and using the feature flag framework to check the status, otherwise you'll likely need to override this in two places.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
CI.
ADDITIONAL INFORMATION