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

Cleaning up removed rollouts #51

Merged
merged 5 commits into from
Apr 30, 2019
Merged

Cleaning up removed rollouts #51

merged 5 commits into from
Apr 30, 2019

Conversation

rnnds
Copy link

@rnnds rnnds commented Apr 8, 2019

Problema

Quando uma feature é removida do arquivo de configuração e ela ainda possui registros salvos no storage, não é mais possível fazer a remoção deste registro através de uma chamada da própria API do feature flagger.
Esse problema foi reportado na issue #36

Solução

A solução proposta nesse PR considerou alguns pontos.

  • O unrelease feito hoje é uma ação manual, o que permite que com o tempo venha a existir vários registros no storage não sincronizados com o arquivo de configuração. Isso acontece sempre que um dev remove uma feature e não faz um unrelease antes de atualizar a versão em produção. Para "lembrar" depois quais features foram removidas o dev terá que recorrer ao histórico do git para encontrar quais foram as features removidas do arquivo de configuração.
  • O arquivo de configuração pode conter features relacionadas a várias "models" do sistema, então para identificar quais "features" não existentes no arquivo de configuração ainda estão persistidas no storage é necessário que o dev possa buscar por model ou de maneira geral, para não obrigar o dev ter que fazer várias buscas por várias models.
  • A configuração a ser respeitada deve ser a existente no arquivo de configuração. Neste caso todo registro persistido no storage que não constar no arquivo de configuração é considerado um "lixo".
  • A ação de remoção de um registro desvinculado da configuração deve ser "explicita", ou seja, o dev deve executar uma chamada de método explicitando qual feature deseja remover, isso porque fazer essa ação de forma automática tem um risco com relação ao feature_flagger estar conectado a um mesmo storage/namespace. Supomos que quem usa o feature flagger tem duas apps usando mesmo storage/namespace. Cada app terá um arquivo de configuração com as features. Então uma ação automática de remoção poderia apagar informações que não deveriam ser removidas. Claro que o ideal é que não se use o mesmo storage/namespace. Mas por hora a opção foi então fazer com que seja executado uma ação explicitando a remoção e qual registro se deseja remover.

Como solução foi incluído:

Na model

  • Método na model para listar os registros "soltos" no storage.
Account.detached_feature_keys
retorno:
['email_marketing:alternative_email_flow']
  • Método para remover um registro vinculado a model. Nesse caso como a feature não existe mais no arquivo de configuração é necessário informar a chave completa. O método valida se a chave não está no arquivo de configuração.
Account.cleanup_detached(:email_marketing, :alternative_email_flow)

Em um "Manager"

  • Foi criado uma classe geral para fazer ações independentes da model

  • Método no manager para listar os registros "soltos" no storage, independente de model

FeatureFlagger::Manager.detached_feature_keys
retorno:
['account:email_marketing:alternative_email_flow']
  • Método para remover um registro. Nesse caso como a feature não existe mais no arquivo de configuração é necessário informar a chave completa. O método valida se a chave não está no arquivo de configuração.
FeatureFlagger::Manager.cleanup_detached(:account, :email_marketing, :alternative_email_flow)

@rnnds rnnds added the wip label Apr 8, 2019
@rnnds rnnds changed the title Adding method to list removed rollouts from file Cleaning up removed rollouts Apr 8, 2019
@nsoufr
Copy link
Contributor

nsoufr commented Apr 8, 2019

Hi @rnnds

Can you provide more context?

Thank you for your contribution <3

@rnnds rnnds removed the wip label Apr 22, 2019
@rnnds rnnds requested a review from nsoufr April 22, 2019 16:57
@nsoufr
Copy link
Contributor

nsoufr commented Apr 29, 2019

reviewing @rnnds

@nsoufr
Copy link
Contributor

nsoufr commented Apr 29, 2019

Awesome pull request @rnnds and @isaacsouza.

Thank you so much.

It would be nice to have two things more:

  • Update README explaining how to clean up;
  • Create another issue addressing the implementation of a Rake to allow users to clean-up more easily.

Again, thank you, guys!

Copy link
Contributor

@nsoufr nsoufr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@nsoufr nsoufr merged commit 00971a2 into master Apr 30, 2019
@nsoufr nsoufr deleted the clean-up-removed-rollouts branch April 30, 2019 12:29
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

Successfully merging this pull request may close these issues.

3 participants