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

Use case for Feature#remove vs. Feature#disable? #296

Closed
mildmojo opened this Issue Oct 26, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@mildmojo

mildmojo commented Oct 26, 2017

I'm using flipper with the ActiveRecord adapter, and I was looking for the ability to disable only the boolean gate (as in #139), but found Feature#disable wipes all the feature's gates from the DB.

What's the intended use case for #remove vs. #disable? I understand #remove deletes the feature record, too, but that record only has a couple of housekeeping fields on it. In my limited testing, the Feature API seems to behave the same whether a feature has no gates or doesn't exist at all. At least for my use case, I think I could use #remove instead of #disable 100% of the time and keep a cleaner DB.


The enable/disable asymmetry with no #disable_boolean threw me for a loop. I'm planning to throw some ugliness in a rake task for this:

# feature.rake

namespace :feature do
  namespace :enable do
    desc 'undo a system-wide "enable"; restores previously-allowed actors/groups'
    task :undo, [:feature_name] => :environment do |t, args|
      args = validate_args(t, args)
      Flipper::Adapters::ActiveRecord::Gate.where(
        feature_key: args[:feature_name],
        key: "boolean"
      ).delete_all
    end
  end
end
@jnunemaker

This comment has been minimized.

Owner

jnunemaker commented Oct 27, 2017

What's the intended use case for #remove vs. #disable?

  • disable removes the gates which reverts Feature#enabled? to default behavior (disabled).
  • remove removes the gates and removes the feature from the set of known features. The set of known features is mostly only used for the open source web UI.

I often disable features, but want them to remain in the web UI until I'm completely done with them. At that point I do a remove. The main difference is about whether or not you are completely done with the feature or if you are just temporarily disabling it.

Hope that helps! Let me know if you have any other questions.

The enable/disable asymmetry with no #disable_boolean threw me for a loop.

I'm not sure I follow this. Do you mean you expected that since there is enable/disable there would also be enable_boolean and disable_boolean methods? Or something else? Happy to explain, just wanted to confirm what you were asking so I don't answer the wrong question.

@jnunemaker jnunemaker closed this Oct 27, 2017

@jnunemaker

This comment has been minimized.

Owner

jnunemaker commented Oct 27, 2017

If you don't mind, I would love to know what you are using flipper for as well and how it is working out for you (what went well, what was hard other than what you mentioned here). If you aren't comfortable leaving that information here in a public comment, feel free to email me directly (it shows up on my profile page). I'm just always curious. :)

@mildmojo

This comment has been minimized.

mildmojo commented Oct 27, 2017

Ah, I hadn't considered the web UI. Thanks.

Sorry, I was unclear about asymmetry. I meant how enable adds one gate (boolean) to a feature's gate list, but disable removes all gates. Add-one/remove-all vs. add-one/remove-one. I understand and agree with the reasoning behind the behavior, but I didn't expect remove-one (boolean) to be excluded by the core design.

I'm using flipper for staged feature rollout and masking in-development features on a Rails project at work (with flipper-rails). Enable feature for internal users, enable for a few trusted external users for feedback, enable for the world. We haven't talked about using it for A/B testing yet, but that could come later.

My secret hope is that it lets me merge features in stages with smaller pull requests for easier review and QA without having to expose a half-baked feature to end users.

I'm still at the beginning stages, so I don't have a lot of likes/dislikes yet. The Gates documentation was really helpful for basic usage patterns. I dug around in the REPL to explore the rest of the API.

One issue I had to work around was that double-enabling a feature for an actor or group threw a Postgres exception for violating the index_flipper_gates_on_feature_key_and_key_and_value unique constraint on the gates table. So I've got an enable_once wrapper that does a check-and-set to avoid the exception in the initializer that sets up basic rules and the rake tasks that perform administration.

Another was that a feature object's name can be a symbol or a string depending on how it was first referenced; it's safer to use string-normalized key instead for code that asks things like "does this feature's group list already include this group".

@jnunemaker

This comment has been minimized.

Owner

jnunemaker commented Nov 27, 2017

Sorry, I was unclear about asymmetry. I meant how enable adds one gate (boolean) to a feature's gate list, but disable removes all gates.

Yeah this was a tough call early on in the project. Imagine this scenario:

  1. enable feature for staff group
  2. enable feature for 25% of actors
  3. disable feature

Let's say step 3 sets boolean = false (instead of clearing all the gates). That wouldn't work for Flipper, since flipper checks all gates and the enabled for staff and 25% of actors would remain enabled. What I did originally when I was setting boolean to false is if boolean was false, the result was false no matter what. This solved that issue but created a new one.

Imagine now that we have in addition to the steps above, another one:

  1. enable feature for single actor (yourself)

What do you expect after step 4? I expect that the feature is enabled for me and me alone. If we leave boolean = false as a short circuit that would mean that any gate enables following the disable (setting boolean = false) would continue to short circuit. So the next thought is let's remove the boolean = false when any future enablements happen like step 4. That creates a new blow expectation though, where the feature is enabled for yourself (single actor), but the previous enablements are also enabled (25% of actors and staff group). Tricky and not expected!

The most clear expectation to me was if you disable, you completely disable and any future enablements will be the only ones for the feature, which is why disable clears instead of setting boolean = false. That is a lot of context, so let me know if I can clear anything up (or if any better ideas come to you which I'm always open to).

My secret hope is that it lets me merge features in stages with smaller pull requests for easier review and QA without having to expose a half-baked feature to end users.

Definitely a good idea and a perfect use of flipper.

One issue I had to work around was that double-enabling a feature for an actor or group threw a Postgres exception for violating the index_flipper_gates_on_feature_key_and_key_and_value unique constraint on the gates table.

Interesting, I would have expected that to just ignore/fail silently. I'll add a test case.

Another was that a feature object's name can be a symbol or a string depending on how it was first referenced; it's safer to use string-normalized key instead for code that asks things like "does this feature's group list already include this group".

Yeah, you can always use strings or symbols, but databases don't have support for symbols so thus key (which is used to store in adapter level) is a string. I find symbols aesthetically more pleasing in code for stuff like this and generally that is all I used. Rarely do I have to use key or the string version.

@mildmojo

This comment has been minimized.

mildmojo commented Nov 30, 2017

Yeah this was a tough call early on in the project. Imagine this scenario:

  • enable feature for staff group
  • enable feature for 25% of actors
  • disable feature

Imagine now that we have in addition to the steps above, another one:

  • enable feature for single actor (yourself)

What do you expect after step 4? I expect that the feature is enabled for me and me alone.

I expect that the feature remains disabled for everyone, including you. I've been thinking of the boolean gate as a kind of global control. When asking, "is this actor allowed this feature," I've been thinking that it also implies the question, "is this feature enabled for the platform," which is answered by the boolean gate. But my mental model may not match the implementation or intent.

Your expected behavior for step 4 feels like a stack model, which would also work for me. Push group, push percentage, push boolean, push actor. Traverse the stack from top to bottom until you hit a boolean gate (inclusive) or the bottom of the stack. If any of the rules allow access, the flag evaluates to true. Removing the boolean gate would let the logic consider the old rules that were below the gate on the stack. That may be harder for humans to keep track of, though.

Thanks for fixing the double-enable exception! 👊

Yeah, you can always use strings or symbols, but databases don't have support for symbols so thus key (which is used to store in adapter level) is a string. I find symbols aesthetically more pleasing in code for stuff like this and generally that is all I used.

I was trying to use symbols for everything, too, but found that a feature I created with a symbol name could return a symbol or a string for its .name property, requiring conversion for comparison. If I created the feature by symbol name in the current Ruby session, .name returned that symbol. If the feature was created in a previous session and loaded from the DB, .name returned a string. I understand why it happens, I just wasn't expecting it. I was only inspecting feature names as a workaround for the double-enable exception, so I won't need to do that anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment