Skip to content
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

Re-enable DRUID_IS_ACTIVE flag #8482

Conversation

willbarrett
Copy link
Member

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

The DRUID_IS_ACTIVE configuration option was removed from the codebase at some point. Druid's preferred connection method is via the SQL interface, so this change is necessary in order to deprecate the Druid API-based connection methods in Superset.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2019-10-28 at 2 56 07 PM

TEST PLAN

  • Boot Superset with the configuration option DRUID_IS_ACTIVE set to True, ensure Druid options appear in the "Sources" menu.
  • Boot Superset with the configuration option DRUID_IS_ACTIVE set to False, ensure Druid options are absent from the "Sources" menu.

ADDITIONAL INFORMATION

  • Has associated issue: Make pydruid truly optional #8463 - I decided to split this change out of that PR for clarity and simplicity.
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

REVIEWERS

@mistercrunch @dpgaspar

@codecov-io
Copy link

codecov-io commented Oct 30, 2019

Codecov Report

Merging #8482 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8482      +/-   ##
==========================================
+ Coverage   66.52%   66.53%   +<.01%     
==========================================
  Files         449      449              
  Lines       22595    22600       +5     
  Branches     2367     2367              
==========================================
+ Hits        15032    15037       +5     
  Misses       7425     7425              
  Partials      138      138
Impacted Files Coverage Δ
superset/connectors/druid/views.py 69.12% <100%> (+0.2%) ⬆️
superset/assets/src/SqlLab/actions/sqlLab.js 60.45% <0%> (ø) ⬆️
superset/views/core.py 72.17% <0%> (+0.01%) ⬆️
superset/sql_lab.py 77.93% <0%> (+0.31%) ⬆️

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 18c6d17...a9cd6a2. Read the comment docs.

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

LGTM, @craig-rueda heads up that this may merge conflict with your big PR

@mistercrunch mistercrunch merged commit 1ccfa4f into apache:master Oct 31, 2019
@mistercrunch mistercrunch deleted the wbarrett/re-enable-deprecated-druid-config branch October 31, 2019 05:16
@mistercrunch
Copy link
Member

thujbsup

graceguo-supercat pushed a commit that referenced this pull request Nov 13, 2019
* Re-enable DRUID_IS_ACTIVE flag

* Make CI happy
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 0.36.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants