Add provider spec #41
Conversation
| > In cases of normal execution, the `evaluation details` structure's `reason` field **MUST** contain the value of the `reason` field in the `flag resolution` structure returned by the configured `provider`. | ||
|
|
||
| ##### Requirement 1.15 | ||
|
|
||
| > In cases of normal execution, the `evaluation details` structure's `error code` field **MUST** not be set, or otherwise must contain a null or falsy value. | ||
| > In cases of normal execution, the `evaluation details` structure's `error code` field **MUST** contain the value of the `error code` field in the `flag resolution` structure returned by the configured `provider`. | ||
|
|
||
| ##### Requirement 1.16 | ||
|
|
||
| > In cases of abnormal execution, the `evaluation details` structure's `error code` field **SHOULD** identify an error occurred during flag evaluation, with possible values `"PROVIDER_NOT_READY"`, `"FLAG_NOT_FOUND"`, `"PARSE_ERROR"`, `"TYPE_MISMATCH"`, or `"GENERAL"`. | ||
| > In cases of abnormal execution, the `evaluation details` structure's `error code` field **MUST** identify an error occurred during flag evaluation, having possible values `"PROVIDER_NOT_READY"`, `"FLAG_NOT_FOUND"`, `"PARSE_ERROR"`, `"TYPE_MISMATCH"`, or `"GENERAL"`. |
There was a problem hiding this comment.
Some minor changes here. the main idea is that the evaluation layer should basically be reflecting the data/errors coming up from the provider.
Changed 1.16 to a "MUST" because the evaluation layer MUST never throw, so it's up to it to decide on an the error code and populate it, even if the provider fails to.
There was a problem hiding this comment.
does this preclude a provider from providing an error code that's not in the set specified here?
There was a problem hiding this comment.
I think that needs to be considered. If we want to allow authors to actually be able to write conditional logic around these errors, they need to be rigorously defined in the API (but then we'll lose fidelity mapping them from the underlying SDKs). If we only need errors for debugging, then they can be more free-form. It may merit further discussion.
toddbaert
requested review from
justinabrahms,
agentgonzo,
beeme1mr,
dabeeeenster,
dyladan and
moredip
|
Do we want to specify any semantics around how a provider should handle transient errors on their side. Like if there's a network connectivity issue, should to provider immediately fail, or have a retry/fallback mechanism. One option is to leave this entirely unspecified and up to each provider. Another would be to give a provider implementer some guidance or suggestions - e.g. make a best effort to handle transient failure, but don't hang evaluation for "a long time" (whatever that means). |
specification/types.md
Outdated
| @@ -30,7 +30,16 @@ A structure containing the following fields: | |||
| - reason (string, optional) | |||
| - variant (string, optional) | |||
|
|
|||
| ### evaluation options | |||
| ### Flag Resolution | |||
There was a problem hiding this comment.
again, Flag Resolution is ambiguously either a verb or a noun. Would Evaluation Result or Evaluation Outcome be better?
There was a problem hiding this comment.
How about ResolutionDetails? It's consistent with EvaluationDetails, and in the context of this type, I don't actually know if the verb/noun confusion is really as problematic. Whether you think of it as a verb or a noun, these are the details associated with it.
I'm going to update it with ResolutionDetails. I'm totally open to changing the term "resolution" entirely. I'd simply like to have a term that denotes solely the aspect of the flag evaluation that the provider does (in opposition to the entire evaluation which involves hooks, the client, etc).
specification/types.md
Outdated
| @@ -30,7 +30,16 @@ A structure containing the following fields: | |||
| - reason (string, optional) | |||
| - variant (string, optional) | |||
|
|
|||
| ### evaluation options | |||
| ### Flag Resolution | |||
There was a problem hiding this comment.
As I read this I'm a bit confused as to why we have two distinct structures with the only difference being that one has the flag key and one doesn't. I'm sure there's a good reason, but maybe for the sake of the reader we should give the reasoning here too?
There was a problem hiding this comment.
It's primarily because the Flag Resolution structure is the thing returned from a provider (and completely invisible to the application author, they don't care about providers) while the Evaluation Details is what the client returns in the "Detailed" evaluation methods. It's almost certain that they will diverge further. In fact, in another draft the Evaluation Details also contained a evaluated hooks field that specified all the hooks that had run in the current evaluation (which the provider would know nothing about).
I hope that makes sense. If it doesn't let me know.
I think it's a good point to mention something explaining this, as you say. I'll add something terse and appropriate that conveys this.
This might be relevant to the "event" discussion. Most SDKs have readiness events, etc. We could build an events API which might cover both cases. I think for now, I would err on the side of not defining this until we need to - but I'm not sure. If there's a pressing use-case, we can talk more about it. |
|
@moredip any additional concerns after my updates to your comments? I think we have 2 open questions represented here, which I may create other issues for:
I don't think either of these are a blocker for this, but let me know if you disagree. |
Nope, I think it's good |
Co-authored-by: Justin Abrahms <jabrahms@ebay.com> Co-authored-by: Pete Hodgson <github@thepete.net> Co-authored-by: Steve Arch <sarch@cloudbees.com> Co-authored-by: Michael Beemer <michael.beemer@dynatrace.com>
|
Squashed, added co-authors. |
Added provider spec, with minor associated changes to the existing flag-evaluation spec file.