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

Allow making actors disabled when feature is globally turned on #82

Closed
thenonameguy opened this issue Sep 4, 2015 · 11 comments
Closed

Comments

@thenonameguy
Copy link

In the current UI there is no way to disable for site administrators to disable a certain feature for a given user/account.

@jnunemaker
Copy link
Collaborator

See caveat 2 here: https://github.com/jnunemaker/flipper/blob/1c1b18763190dcfda6df41133ceb6aa63c53c35e/docs/Caveats.md

Flipper is for explicitly letting people through gates, not keeping them out. That said, can you further describe your use case? Maybe there is a currently supported way of doing what you need to do or maybe it makes sense to add.

@thenonameguy
Copy link
Author

We are guarding one of our product features with a feature flipper. After enabling it for a few actors, we enabled it globally, but one of our customers did not want to expose it for their accounts users. Our site admins did not find a way to disable it only for them on the UI, other than turning the feature off and listing every actor manually as enabled, except the problematic one.

@jnunemaker
Copy link
Collaborator

Ah, yeah so not a common case probably. One other way to solve it could be a group. You could make a group and enable it like this:

Flipper.register(:some_group) do |actor|
  !disabled_customer_ids.include?(actor.id)
end

Flipper[:some_feature].disable
Flipper[:some_feature].enable(:some_group)
Flipper[:some_feature].enabled?(disabled_customer) # false
Flipper[:some_feature].enabled?(other_customer) # true

@rmosolgo
Copy link

I just came looking for something like this too:

In the course of rolling out a feature, there are a few specific cases where it doesn't seem to work yet 😖 So I found myself looking for a flow like this:

  • Enable a feature for everyone; THEN
  • Disable the feature for a few actors

I found a workaround, using two different features:

  • "Use Feature X": Enabled
  • "Blacklist Feature X": Enable for a few actors

then do the application logic like this:

def feature_x_enabled?
  if flag_enabled?("Blacklist Feature X")
    false 
  else 
    flag_enabled?("Use Feature X")
  end 
end 

It's a bit tough to have the gate logic split between two, but it provides the behavior I was looking for!

@jnunemaker
Copy link
Collaborator

@rmosolgo thanks for the feedback! Love hearing usecases, especially with examples like that. I've been thinking lately that perhaps the v2 adapters could make this more easily possible as we could just add a new set that gets serialized next to enabled for disabled. Then, with a bit of negation code we would be good to go. I still wonder if the complexity1 is worth it, but if people keep finding that they need it, at least flipper can hide that complexity as best as possible.

[1]. When I refer to complexity, mostly I mean everything has to change. API endpoints will need to return enabled and disabled actors and groups separately. UI will need to support showing enabled and disabled. Instrumentation will probably need to expose that something is enabled but disabled for specific actor instead of just enabled or not.

@rmosolgo
Copy link

I still wonder if the complexity is worth it

I would guess not since there's about one request per year, judging by this thread 😆

I'm happy to "work around" it, and like you said, this is a very fair design goal/constraint:

Flipper is for explicitly letting people through gates, not keeping them out.

And making two features worked fine for me (I was only worried about the support agents who might have had to flip it themselves). But in the end, we had to flip the master "OFF" switch anyways 😩 !

@jnunemaker
Copy link
Collaborator

@rmosolgo appreciate the feedback. I will keep an eye on this thread and if we do start to see an increase we can come back to it.

@galori
Copy link

galori commented Jun 18, 2018

FWIW I would have liked this feature to exist.

@ozydingo
Copy link

ozydingo commented Jan 13, 2021

I may be speaking into a void here, but I also have a use case for this. I am going to implement the workaround of having two gates, one for a blacklist, but it makes using Flipper considerably more awkward.

My use case: we're rolling out a new processing method for video assets. We're going to enabled it by percentage of actors to test the waters. The main fear is the new method will be too slow or fail entirely sometimes. We therefore need to be able to respond to failures or to approaching customer deadlines by forcing problematic files to the old path. It seems our choices are build in custom app logic (unacceptable -- ops team has no control), or use a second gate as a blacklist. Having disabled actor IDs would be a natural solution.

EDIT -- another option would be to roll back to 0% when an issue is detected for a file that needs to go through. This is doesn't pass our bar as it too drastically slows down our dev cycles and data collection for the many files the new method does work on. By having a disable-by-actor, we can push forward with the rollout plan, handle edge cases manually, and, in parallel, work on improving the feature so that those edge cases are no longer problems.

@jnunemaker
Copy link
Collaborator

I may be speaking into a void here, but I also have a use case for this.

Never. I'm always here listening. 😄 I love getting specific use cases like this.

There are two concerns:

  1. Storage: easy. We can just add a new gate with set datatype (DisabledActor or whatever).
  2. Interface: hard. I think the part I've always struggled with is you end up in a weird state when you want to un-disable an actor on the disabled list.

Let's say you enable a % of time. Then you disable_actor "User;1" the customer that is having problems with video processing. How to you remove them from the "not enabled" set?

I guess we could make enable_actor "User;1" remove them from the not enabled set instead of adding them to the enabled set.

Like check out this decision tree:

method not in enabled set / not in disabled set in enabled set in disabled set
enable_actor adds to enabled set does nothing removes from disabled set, does not add to enabled? or does it remove from enabled and add to enabled?
disable_actor adds to disabled set same problem as above, does it remove from enabled and add to disabled or just remove from enabled does nothing

Another option would be to make set datatypes different than the other gates. So instead of enable_actor/disable_actor, you would do add_enabled_actor/remove_enabled_actor and add_disabled_actor/remove_disabled_actor.

This would be a breaking change. Probably minor and a quick search/replace for people. But it wouldn't be minor from how you think about Flipper, since thus far it is all about is anything enabled.

@ozydingo
Copy link

I see the challenge. I think it boils down to:

  1. Backward compatibility while having the new feature make intuitive sense
  2. Confusion about removing enabled vs disabling. I.e.: is it worth it?

I would need to understand the gate classes better to have a more informed opinion here. Maybe I'll dive in when i have a bit more time. For now I'm taking what you said to imply that we'd heavily prefer to stick to the same interface with enable_actor and disable_actor methods, short of a more fundamental rethink of the set datatypes. I think the mapping of the "enable" and "disable" names to adding and removing to a set lock us in a bit. From scratch I could imagine using "enable", "remove", and "disable", where remove_actor does what disable_actor does now, and disable_actor adds them to a blacklist. But that's a screwy breaking change to introduce!

I believe naming is important, so I'm thinking hard of what else could take the place of that third option without adding confusion to the existing disable_actor. Maybe blacklist_actor successfully implies that you're blocking the actor from ever getting enabled? disable is still slightly confusing here, but could an option to help clarify things be to (1) add remove_actor as doing the same thing as disable_actor, and (2) deprecate disable_actor with a message about remove_actor so that it's obvious what it does and it sets up the ability to make the breaking change later in a major version bump?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants