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

Add helper class for implementing feature flags #413

Open
joshmcarthur opened this issue Jan 24, 2023 · 5 comments
Open

Add helper class for implementing feature flags #413

joshmcarthur opened this issue Jan 24, 2023 · 5 comments
Labels
discuss Discussion required

Comments

@joshmcarthur
Copy link
Contributor

Quite often, established projects will need to implement feature flags. I've seen feature flags in two main categories:

  1. Implementation switching flags - used to transition between one code path to another. These tend to have a finite lifetime.
  2. Availability flags - used to determine if a feature is active or not. These tend to be more incorporated into the codebase.

While there are lots of libraries that add this feature for us, we don't often need that level of complexity.

I'm opening this issue to gauge support for us adding a simple implementation of feature flagging into the template, so that we can do it consistently. This feature flagging probably only needs to support environment-variable based flags, and ideally we can include some testing support so that tests can override flags to test particular paths.

This is something that seems simple, but can be done in a million different ways, so having a central implementation of a class + an rspec support file might be useful.

A minimal implementation would be:

# app/models/feature_flag
class FeatureFlag
  def enabled?(feature_name)
    ENV.fetch("FEATURE_FLAG_#{feature_flag.upcase}", false).match?(/\Ayes|t/gi)
  end
end

# spec/support/feature_flags.rb
RSpec.configure do |config|
  config.before(:each, :feature_flags) do |example|
    feature_flags = example.metadata.fetch(:feature_flags)
    feature_flags.each { |flag, enabled| allow(FeatureFlag).to receive(:enabled?).with(flag).and_return(enabled)
  end
end

This would allow features to be enabled or disabled using env vars, with the ability to override these for specific tests:

FEATURE_FLAG_HIGH_PERFORMANCE=true bundle exec rails console

# or

FEATURE_FLAG_HIGH_PERFORMANCE=yes bundle exec rails console

# or

FEATURE_FLAG_HIGH_PERFORMANCE=false bundle exec rails console

# or

it "runs in high performance mode", feature_flags: { high_performance: true, throttle: false } do
  expect(subject).to be_high_performance
end
@robotdana
Copy link
Contributor

robotdana commented Jan 24, 2023

consider having a list of defined feature flags and raise if it's not in the defined list, to guard against typos

something like

class FeatureFlag
  VALID_FLAGS=%w{
    high_performance
    whatever
    another
  }
  def enabled?(feature_name)
    raise ArgumentError, "Unrecognized flag #{feature_name}" unless VALID_FLAGS.include?(feature_name.to_s)
    
    ENV.fetch("FEATURE_FLAG_#{feature_flag.upcase}", false).match?(/\Ayes|t/gi)
  end
end

@robotdana
Copy link
Contributor

(also please allow "1" to be a true value)

@robramsaynz
Copy link
Contributor

Broadly speaking I like the direction of this. I've used feature flags in the past, and the systems were generally over the top complication for most of what we do (eg creating DB-columns to track per-user system settings).

@eoinkelly
Copy link
Contributor

I'm currently leaning towards just making Flipper our default.

Having lived with Flipper for a while now on a recent project, the admin UI it creates has been surprisingly useful because it allows clients to manage flags during UAT testing. Also changing feature flag values via rails console is less faff than changing ENV vars (especially in AWS hosted apps). Basically, I like Flipper a lot more than I thought I would.

@eoinkelly
Copy link
Contributor

Adding a "discuss" tag because I think I can make a reasonable case for just using Flipper.

@eoinkelly eoinkelly added the discuss Discussion required label Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Discussion required
Projects
None yet
Development

No branches or pull requests

4 participants