flagr icon indicating copy to clipboard operation
flagr copied to clipboard

Unable to re-use flag key from deleted flag

Open kenli-rubrik opened this issue 5 years ago • 4 comments

Expected Behavior

Suppose I specify flag_key="blah" for flag 1. I delete flag 1, and then try to create a new flag with flag_key="blah". I would expect that I can create the new flag successfully since the flag key is no longer being used.

Current Behavior

Instead, I get the error:

Error 1062: Duplicate entry 'thisisatestflag' for key 'idx_flag_key'

Possible Solution

This is likely due to GORM using soft deletes, so we're unable to add the new flag because the key still exists in the index.

Temporary workaround we're planning to use is to modify the flag key to a dummy value before deleting any flags. A real solution could involve detecting that the flag key exists under another flag and either:

  1. Stripping the key from the old flag, and continue creating the new flag.
  2. Reviving the old flag (and clearing other related data like segments, variants, etc?). Related to https://github.com/checkr/flagr/issues/215

Steps to Reproduce (for bugs)

  1. Create a flag with a know key
  2. Delete the flag from step 1
  3. Attempt to create a new flag with the same flag key

Context

We have some testing utils that will create a temporary flag (identified by key), run some code that depends on the flag, and then delete the flag. However consecutive attempts to re-use the same key fails due to this issue.

kenli-rubrik avatar May 15 '19 17:05 kenli-rubrik

I actually like that we keep the unique index including deleted flags, this makes restoring the deleted flags logic simple and clean in the future.

Is it possible to create temporary flags with a randomly generated key for each test? If not, I would prefer resetting the data storage of flagr so each test you can have a clean setup.

zhouzhuojie avatar May 15 '19 22:05 zhouzhuojie

I think if we had the ability to restore flags, that would be mostly sufficient. As long as we could easily detect that a flag was deleted and revive it instead of creating it again.

To expand on the context a bit more, it's not just testing where this edge case comes up. We also have a script that synchronizes flag state from a YAML file (deleting any flags that no longer exist). If a developer removes a flag and then tries to re-add it later, the script will fail. Or another example is that a flag is added to the YAML file, the commit is reverted, and sometime later the flag is added again.

kenli-rubrik avatar May 15 '19 22:05 kenli-rubrik

Got it, I was thinking of syncing flags recently and added features like exporting the eval cache to JSON files. For example, we are using the exported JSON files in our test and staging environment. It basically makes the flagr instance into eval_only (read-only) mode, so you don't even need to care about deleted flags, since there's no DB and no writes are allowed, and the source of truth will the JSON file. I think it may be one of the options for your setup.

related to https://github.com/checkr/flagr/pull/247

https://checkr.github.io/flagr/#/flagr_env There's a new block of comment about the new values of FLAGR_DB_DBDRIVER.

zhouzhuojie avatar May 15 '19 23:05 zhouzhuojie

We can also try YAML as the serialization format, definitely open to contribution.

zhouzhuojie avatar May 15 '19 23:05 zhouzhuojie

Stale issue message

github-actions[bot] avatar Aug 26 '22 21:08 github-actions[bot]