Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: handle context-dependent feature flags in CLI #12088

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

betodealmeida
Copy link
Member

SUMMARY

@ktmud pointed out in #11803 that GET_FEATURE_FLAG_FUNC might use the app context, breaking the CLI. This PR fixes it.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

Added this to my superset_config.py:

# pylint: disable=invalid-name
def GET_FEATURE_FLAGS_FUNC(feature_flags_dict: Dict[str, bool]) -> Dict[str, bool]:
    from flask import g

    if hasattr(g, "user") and g.user.is_active:
        feature_flags_dict["some_feature"] = g.user and g.user.id == 5
    return feature_flags_dict

The CLI works as expected.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Dec 17, 2020

Codecov Report

Merging #12088 (539ad3b) into master (08b3ebe) will decrease coverage by 3.57%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12088      +/-   ##
==========================================
- Coverage   67.26%   63.69%   -3.58%     
==========================================
  Files         959      959              
  Lines       47241    47232       -9     
  Branches     4629     4629              
==========================================
- Hits        31778    30083    -1695     
- Misses      15345    16965    +1620     
- Partials      118      184      +66     
Flag Coverage Δ
cypress ?
javascript 62.68% <ø> (ø)
python 64.30% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/cli.py 34.50% <0.00%> (-0.29%) ⬇️
superset-frontend/src/explore/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupColors.js 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/chart/ChartContainer.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/reducers/index.js 0.00% <0.00%> (-100.00%) ⬇️
...et-frontend/src/dashboard/containers/Dashboard.jsx 0.00% <0.00%> (-100.00%) ⬇️
...t-frontend/src/dashboard/containers/SliceAdder.jsx 0.00% <0.00%> (-100.00%) ⬇️
...-frontend/src/visualizations/presets/MainPreset.js 0.00% <0.00%> (-100.00%) ⬇️
...tend/src/dashboard/containers/DashboardBuilder.jsx 0.00% <0.00%> (-100.00%) ⬇️
... and 158 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08b3ebe...539ad3b. Read the comment docs.

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@betodealmeida betodealmeida merged commit dd0c531 into apache:master Dec 17, 2020
amitmiran137 pushed a commit to simcha90/incubator-superset that referenced this pull request Dec 18, 2020
* upstream/master: (55 commits)
  feat(explore): time picker enhancement (apache#11418)
  feat: update alert/report icons and column order (apache#12081)
  feat(explore): metrics and filters controls redesign (apache#12095)
  feat(alerts/reports): add refresh action (apache#12071)
  chore: add latest tag action (apache#11148)
  fix(reports): increase crontab size and alert fixes (apache#12056)
  Small typo fix in Athena connection docs (apache#12099)
  feat(queries): security perm simplification (apache#12072)
  feat(databases): security perm simplification (apache#12036)
  feat(dashboards): security permissions simplification (apache#12012)
  feat(logs): security permissions simplification (apache#12061)
  chore: Remove unused CodeModal (apache#11972)
  Fix typescript error (apache#12074)
  fix: handle context-dependent feature flags in CLI (apache#12088)
  fix: Fix "View in SQLLab" bug (apache#12086)
  feat(alert/report): add 'not null' condition option to modal (apache#12077)
  bumping superset ui to 15.18 and deckgl to 0.3.2 (apache#12078)
  fix: Python dependencies in apache#11499 (apache#12079)
  reset active tab on open (apache#12048)
  fix: improve import flow UI/UX (apache#12070)
  ...
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XS 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants