You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[OFREP] Should variant and reason tolerate what the spec makes optional? #418
Two questions about the same three lines in ofrep/__init__.py, both about optional fields being indexed unconditionally. Asking rather than reporting, in case either is a deliberate strictness choice.
KeyError: 'variant', surfacing to the application as GENERAL. Note value is already handled correctly for this response — OFREP's codeDefaultFlag schema omits value on purpose ("The provider must use the code default value when processing this response"), and data.get("value", default_value) does exactly that. So the response is understood apart from the variant.
This is not specific to disabled flags: any response omitting variant would do it.
Question: should this be data.get("variant"), or is there a reason to require the field?
2. Reason[...] is an enum lookup by name
Reason[data["reason"]] raises KeyError for any reason outside the nine members of Python's Reason enum. But Requirement 2.2.5 permits a provider to populate reason with one of the listed values "or some other string indicating the semantic reason for the returned flag value", and types.md adds:
The reason should not be limited to the reasons enumerated above. It can be any of the pre-defined reasons, or any string value.
The Python SDK models this correctly — FlagEvaluationDetails.reason is typed str | Reason | None. So an OFREP server returning a vendor-specific reason string appears to crash this provider on a response the specification explicitly allows.
I have not observed this one in the wild — flagd only returns standard reasons — so it is inference from the code, not a reproduction. Flagging it because it is the same line and the same class of thing.
Question: should the reason fall back to the raw string when it is not a known member, rather than raising?
Context
Found while building the cross-language provider conformance suite proposed in open-feature/spec#417. A new gated @disabled-flags capability exposed (1); (2) came from reading the surrounding lines. For calibration, the Go and JavaScript OFREP providers pass the same scenarios, so this does not look like a limitation of OFREP.
We recently created an internal feature flag service and were excited to implement the OFREP spec and get a bunch of client libraries for free. While creating a python sample project using the OpenFeature's python OFREP provider, we noticed that it throws an error when variant field is not in the evaluation response.
It is not a blocker for us because we can easily provide a value for it, but this is an inconsistency between the spec and the python OFREP provider, and one should change IMO.
Activity
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
nick-durcholz-vectorsolutions commentedon Sep 24, 2026
We recently created an internal feature flag service and were excited to implement the OFREP spec and get a bunch of client libraries for free. While creating a python sample project using the OpenFeature's python OFREP provider, we noticed that it throws an error when variant field is not in the evaluation response.
It is not a blocker for us because we can easily provide a value for it, but this is an inconsistency between the spec and the python OFREP provider, and one should change IMO.
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings
docs(ofrep): keep the evidence, link the rules, delete the retellings