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

[BUG] flagd doesn't reindex flags on delete #181

Closed
agardnerIT opened this issue Oct 2, 2022 · 2 comments · Fixed by #182
Closed

[BUG] flagd doesn't reindex flags on delete #181

agardnerIT opened this issue Oct 2, 2022 · 2 comments · Fixed by #182
Assignees
Labels
bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer

Comments

@agardnerIT
Copy link
Contributor

Observed behavior

Changing a flag key while flagd is running doesn't reindex the available flags and the old value is returned until flagd is restarted.

Adding new flags does work

Expected Behavior

Adding and removing flags is entirely dynamic and the JSON file is always the source of truth.

Steps to reproduce

Create a JSON file:

{
    "flags": {
      "foo": {
        "state": "ENABLED",
        "variants": {
          "value": "baz"
        },
        "defaultVariant": "value"
      }
    }
}

Which (correctly) returns baz.

Now change "foo": { to "foo2": { and re-execute the retrieval of foo. I'd expect a "no flag found" error or similar, but I still get baz.

Restart the flagd process and foo is no longer found (which is the correct behaviour).


Alternatively, run flagd and (while it is running) add a new flag called foo3. Now get foo3 and that works. Proving that adding dynamically works but deleting doesn't.

@agardnerIT agardnerIT added bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer labels Oct 2, 2022
@AlexsJones
Copy link
Member

Thank you for submitting the bug, I suspect this is to do with the way we merge JSON schema together.

func (je *JSONEvaluator) SetState(state string) error {

@skyerus @toddbaert thoughts?

@james-milligan james-milligan self-assigned this Oct 3, 2022
@toddbaert
Copy link
Member

Ya I think the merge was originally implemented without really thinking about flags being removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs Triage This issue needs to be investigated by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants