-
Notifications
You must be signed in to change notification settings - Fork 14.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove pydash merge #15435
fix: remove pydash merge #15435
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15435 +/- ##
==========================================
- Coverage 77.14% 76.98% -0.16%
==========================================
Files 975 975
Lines 50662 50662
Branches 6228 6228
==========================================
- Hits 39081 39004 -77
- Misses 11370 11447 +77
Partials 211 211
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Instead of deep-merging all config values, have we considered merging only the selected config keys that actually needed it? |
(cherry picked from commit 83be06d)
SUMMARY
#15405 builds the configuration for Superset using
pydash.objects.merge
to combinesuperset_config.py
intosuperset/config.py
.This fails when a
RESULTS_BACKEND
is configured, sincepydash
tries to deepcopy all the objects, but acachelib.redis.RedisCache
(a valid backend) can't be deepcopied because it has a lock:Additionally, the merge strategy is not what we want to do. For example, we can have languages configured in
superset/config.py
:If a user wants to have only Brazilian Portuguese in their Superset instance they would add to their config:
But the
merge
would bring back all languages, resulting in a config that looks like this:BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION