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.
value = data.get("value", default_value) # tolerant
_typecheck_flag_value(value, flag_type)
return FlagResolutionDetails(
value=value,
reason=Reason[data["reason"]], # (2)
variant=data["variant"], # (1)
flag_metadata=data.get("metadata", {}),
)
value is read with a default; the two below it are not. That asymmetry is what makes me think the others might be oversights rather than intent.
1. variant is optional, but indexed
types.md types it variant (string, optional), and OFREP's evaluationSuccess requires only key and reason.
Observed against flagd-testbed:v3.8.0:
$ curl -s -X POST localhost:8016/ofrep/v1/evaluate/flags/disabled-boolean-flag -d '{"context":{}}'
{"key":"disabled-boolean-flag","reason":"DISABLED","metadata":{}} HTTP 200
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.
Happy to be told either of these is intended.
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.valueis read with a default; the two below it are not. That asymmetry is what makes me think the others might be oversights rather than intent.1.
variantis optional, but indexedtypes.mdtypes itvariant (string, optional), and OFREP'sevaluationSuccessrequires onlykeyandreason.Observed against
flagd-testbed:v3.8.0:KeyError: 'variant', surfacing to the application asGENERAL. Notevalueis already handled correctly for this response — OFREP'scodeDefaultFlagschema omitsvalueon purpose ("The provider must use the code default value when processing this response"), anddata.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
variantwould 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 nameReason[data["reason"]]raisesKeyErrorfor any reason outside the nine members of Python'sReasonenum. But Requirement 2.2.5 permits a provider to populatereasonwith one of the listed values "or some other string indicating the semantic reason for the returned flag value", andtypes.mdadds:The Python SDK models this correctly —
FlagEvaluationDetails.reasonis typedstr | 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-flagscapability 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.Happy to be told either of these is intended.