You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2025/02/19 23:20:35 UTC

[PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

rusackas opened a new pull request, #32318:
URL: https://github.com/apache/superset/pull/32318

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   Following on the heels of https://github.com/apache/superset/pull/32317, and a discussion with @eschutho, we feel that this feature is ready for promotion to being on by default, whether this makes it into 5.0 or a subsequent minor. Again tagging @kgabryje @michael-s-molina @mistercrunch in case there are any concerns here preventing this change.
   
   <!--- Describe the change below, including rationale and design decisions -->
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2672333378

   Shouldn't we mention this in UPDATING.md to let people know it's on by default now?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2675446053

   > Shouldn't we mention this in UPDATING.md to let people know it's on by default now?
   
   I agree that feature flag default changes aren't breaking changes, but should be noted in `UPDATING.md`, where more heads up to the person performing the release is better. It allows them to know about the change in behavior, and prevent it if they decide so.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2675468372

   @sadpandajoe Ephemeral environment spinning up at http://52.13.110.49:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.

michael-s-molina commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2671219527

   @rusackas We had a proposal about this in 4.0 that was blocked because cross-filters worked pretty poorly with horizontal filters when compared to the vertical approach. I don't know if we had a design review since then but it would be good to check before making this the default. @kasiazjc


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-3604296087

   @sfirke I.... don't recall! I think perhaps there were some frustrating test failures, or the feature was deemed not-quite-ready for prime time in another conversation somewhere. 
   
   If we wanna do this, I can try to reopen/recreate it, but I'm not sure if @sadpandajoe @msyavuz @yousoph have any feelings that we shouldn't (yet) flip it on for everyone. 
   
   I suppose a "lazy consensus" thread couldn't hurt if there is any contentiousness about flipping it on when we're not in a major release cycle.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.

sfirke commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-3598796643

   @rusackas I'm wondering what the state of this is - it looks above like there was consensus to make this happen but it got aborted?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.

sadpandajoe commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2675464961

   @rusackas I pushed up a possible fix for the e2e cypress test that's failing. When tags are turned on, we add a `Tags` column between `Status` and `Owners`, so the e2e was failing since it was expecting `Owners` but it was returning `Tags`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.

mistercrunch commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2672222648

   Wondering who's the best person to help, though should be a simple glitch around feature flags handling in tests/builds(?)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas closed pull request #32318: feat(feature flag): Setting `TAGGING_SYSTEM` to True by default
URL: https://github.com/apache/superset/pull/32318


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2670027025

   Whoa... CI is not happy. Not sure yet what can of worms I've opened here :/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(feature flag): Setting `TAGGING_SYSTEM` to True by default [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #32318:
URL: https://github.com/apache/superset/pull/32318#issuecomment-2675454758

   @sadpandajoe Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/13464478934). Action: **up**.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org