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

Unable to re-use flag key from deleted flag #255

Closed
kenli-rubrik opened this issue May 15, 2019 · 5 comments
Closed

Unable to re-use flag key from deleted flag #255

kenli-rubrik opened this issue May 15, 2019 · 5 comments

Comments

@kenli-rubrik
Copy link

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 [feat] restore deleted flags #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.

@zhouzhuojie
Copy link
Collaborator

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.

@kenli-rubrik
Copy link
Author

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.

@zhouzhuojie
Copy link
Collaborator

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 #247

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

@zhouzhuojie
Copy link
Collaborator

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

@github-actions
Copy link

Stale issue message

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

No branches or pull requests

2 participants