[Task #960] Introduce a feature flag system #149
Conversation
Benaaaaa
requested review from
nikajukic,
cilim,
uncoverd and
vr4b4c
as code owners
Move env variables initialization to figaro file Since env variables are now being properly initialized through figaro file Flipper cloud is properly being required so cloud gem is no longer needed as well Add after_initialize block to make sure flags are synchronized upon initialization
README.md
Outdated
| @@ -35,6 +35,16 @@ For build workflow to work, the following secrets must exist (usually set up by | |||
|
|
|||
| For deploy workflows, you need to generate private/public SSH key pairs for each environment. Public key should be added to the server to which you're deploying. Private key should be added as a secret to GitHub and named `SSH_PRIVATE_KEY_#{ENVIRONMENT}`, where `ENVIRONMENT` is replaced with an appropriate environment name (`STAGING`, `PRODUCTION`, etc.). | |||
|
|
|||
| ### Flipper | |||
|
|
|||
| This template uses Flipper for feature flag management. There are two ways for Flipper to function: **Cloud** and **Cloudless**. Cloudless will work out of the box, but if we want Flipper to be communicating with Flipper cloud there are a couple of secrets that need to be set: | |||
There was a problem hiding this comment.
Could you emphasize that the cloud version costs $$
There was a problem hiding this comment.
The cloud version is free though 🤔
And the idea is to be using the cloud version, as such I did not present the UI version implementation, as cloud is free and offers a superset of features
There was a problem hiding this comment.
Oh my bad, I had a vague recollection of them only offering a paid cloud version 😓 Seems like it was introduced in August 2023, great 👍
| FLIPPER_CLOUD_TOKEN: '' | ||
| FLIPPER_CLOUD_SYNC_SECRET: '' |
There was a problem hiding this comment.
This is secrets.yml, so shouldn't this be flipper_cloud_token and flipper_cloud_sync_secret, downcased as other parameters as well?
There was a problem hiding this comment.
It seems a bit weird when reading it, but this is being added to the figaro file. Based on the documentation:
Using Environment Variables
Environment variables provide the easiest way to configure Flipper Cloud. Set a FLIPPER_CLOUD_TOKEN environment variable using .dotenv or any other approach that works with environment variables, and Flipper will recognize it and handle the rest.
FLIPPER_CLOUD_TOKEN=your-cloud-token
FLIPPER_CLOUD_SYNC_SECRET=your-webhook-secret
By simply adding these two to the figaro file, figaro will load them as env variables upon startup. With such a setup, as long as the secrets are setup Flipper Cloud will work out of the box
There was a problem hiding this comment.
I don't think casing matters in practice, but I think we should keep it uniform. Since Figraro docs are using downcase and we followed it in other places, we should keep it consistent here as well, downcased.
There was a problem hiding this comment.
I did introduce it downcased and it did not properly work downcased. Flipper cloud was not being loaded, it seems it is explicitly asking for upcased ENV variables
There was a problem hiding this comment.
E.g. our figaro variables are set downcased:
And we need them to be upcased for Flipper to work
| @@ -1,3 +1,5 @@ | |||
| require 'tty-prompt' | |||
There was a problem hiding this comment.
No bueno, the template shouldn't depend on anything outside of the standard library. Currently, rails new fails if tty-prompt isn't installed beforehand
The template [https://raw.githubusercontent.com/infinum/default_rails_template/feature/960-introduce-a-feature-flag-management-system-to-the-default-rails-template/template.rb] could not be loaded. Error: cannot load such file -- tty-prompt
| FLIPPER_CLOUD_TOKEN: '' | ||
| FLIPPER_CLOUD_SYNC_SECRET: '' |
There was a problem hiding this comment.
I don't think casing matters in practice, but I think we should keep it uniform. Since Figraro docs are using downcase and we followed it in other places, we should keep it consistent here as well, downcased.
template.rb
Outdated
| @@ -727,5 +763,10 @@ def run | |||
| run 'overcommit --sign' | |||
| run 'overcommit --sign pre-push' | |||
|
|
|||
| ## Flipper setup for active record adapter | |||
There was a problem hiding this comment.
I don't think we need this comment if the logic is self-explanatory.
Task: #960
Aim
We need to introduce a feature flag management system to the default Rails template.
Solution
Introduce flipper to the project
Do not set ! on secrets regarding flipper cloud, as they are not necessary for Flipper to be working
Update the readme regarding Flipper