open-feature/playground

few questions on the API

justinabrahms opened this issue · 7 comments

looking at https://github.com/open-feature/sdk-research/blob/main/packages/openfeature-js/src/lib/types.ts, a few questions came up for me.

export type FlagEvaluationVariationResponse = FlagEvaluationResponse & {
  stringValue?: string;
  boolValue?: string;
  numberValue?: string;
};

I would have expected boolValue to be a bool, etc?

FlagEvaluationRequest should require a default, I believe, in the case of connectivity issues.

There's no way to understand why you got one of those results. e.g. did you match a rule? was it a fallthrough case? Useful for debugging.

Is there a use-case where folks don't know the data type their requesting? I would have guessed that:

  getVariation(
    id: string,
    context?: Context
  ): Promise<FlagEvaluationVariationResponse>

would actually return something like Promise<FlagResponse<Boolean>> instead.

I'm not clear why there's so much overlap on the data inside getVariation and evaluateFlag.

@justinabrahms, good catch! Yes, I messed up the types on the FlagEvaluationVariationResponse. I'll fix that ASAP.

I think handling fallback scenarios should be part of a bigger discussion. In JavaScript, it would be nice to pass in an optional callback. One of the arguments could be an error type. The developer would have more context around the issue so they could handle it how they see fit.

For example:

isEnabled("test-feature", { user: "test-user" }, (errType) => {
 console.log(errType)
 return false;
})

We could also accept a default value like this.

isEnabled("test-feature", { user: "test-user" }, false);

Perhaps it would be better to have methods that explicitly handle data types instead of having a single method. My thought around the single method is that additional data could be included in the response like a flag evaluation summary. What would be your preference?

getVariation is developer facing and evaluateFlag is for the provider.

The SDK we're looking at is a mixture. Either a value-oriented result or a context oriented result.

assert true == isEnabled("test-feature", { user: "test-user" }, false);  // default happy case w/ a match
assert false == isEnabled("test-feature", { user: "test-user" }, false); // case where there was a connectivity issue

versus..

result = contextyApi("test-feature", { user: "test-user" }, false);
assert true == result.getValue()
assert enum.CONNECTIVITY_ISSUE == result.getReason()

That getReason is a terrible API, but hopefully it communicates that you can interrogate how we came to the decision that the value was true.

When would getReason be used? Would it be for troubleshooting only or would you use it in your application logic?

Perhaps something like LaunchDarkly's evaluation reasons would work well.

We'd primarily use it for logs/debugging purposes.

I think the current spec addresses most of these concerns. Please re-open if you disagree.