launchdarkly / go-server-sdk

LaunchDarkly Server-side SDK for Go
Other
41 stars 17 forks source link

Expose client.variation method #73

Closed phoenix2x closed 2 years ago

phoenix2x commented 2 years ago

Is your feature request related to a problem? Please describe. We have a use case where we don't know the flag type in advance. Unfortunately, ldclient exposes only type-specific XXXVariation methods. It would be nice to have a way to examine type ourselves.

Describe the solution you'd like The simplest one is to expose variation method that returns (ldreason.EvaluationDetail, error) and allows users to deal with types.

Describe alternatives you've considered Right now we have to use a workaround. Call StringVariationDetail and if the error kind is WRONG_TYPE call the following method for example BoolVariationDetail, etc.

cwaldren-ld commented 2 years ago

Hi @phoenix2x , thank you for the request.

Let me first ask - have you tried using the JSONVariation API, which may satisfy your requirements?

phoenix2x commented 2 years ago

Hi @cwaldren-ld,

Thank you for the quick response. Yes, JSONVariation works for us.

Do you think it's a workaround that relies on implementation detail(the lack of type check in the JSONVariation method) or it's a feature? When I see the JSONVariation method, I think about JSON flag variation in launchdarkly UI:) Curious if you're planning to expose a generic Variation method.

Thank you.

cwaldren-ld commented 2 years ago

Hi @phoenix2x , JSONVariation is a legitimate feature of the library, please feel free to use it! (docs #1, docs #2).

The return value, which is an ldvalue.Value, can hold any value representable within the LaunchDarkly system.

As for generics, are you referring to the Go language's new generics feature, or something else?

phoenix2x commented 2 years ago

Hi @cwaldren-ld , I guess I need to clarify myself:) The confusion comes from an uncanny resemblance between XXXVariation methods and variation types in the app.launchdarckly.com. It's not very intuitive that JSONVariation method can be used to evaluate Boolean variation type for example. So I was asking to have Variation(without type prefix) method that quickly tells the user that it can be used for any configured variation type.

image image
eli-darkly commented 2 years ago

As the documentation says, the JSONVariation method can be any JSON type. Boolean, number, and string are all JSON types. The fact that the JSONVariation method doesn't do type checking is not an implementation detail; it's the definition of the method, so yes you can rely on it.

I think the source of the confusion here is that when the dropdown menu in the UI says "JSON", it really means "JSON data that isn't one of those simple types." There's a separate option for that simply because otherwise if you typed something like ["a"], it wouldn't know whether you meant "a JSON array containing a string value 'a'", or "a string consisting of the characters [, ", a, ", ]", either of which would be valid as a flag variation value.

eli-darkly commented 2 years ago

I myself would prefer to have the UI present this differently, and it's been raised as an issue before— as has the idea of renaming the SDK methods to something like "AnyTypeVariation". So I don't mean to say that this is not confusing or that there's no reason for you to have interpreted it that way. However, technically it is correct to say that a boolean value is a JSON value; fundamentally the LD data model, when it comes to flag variations, is defined in terms of JSON and that is one of the basic features of the JSON spec.

phoenix2x commented 2 years ago

Hi @eli-darkly,

Sure, I'm not saying it's not correct. Just saying that you could spare your time doing more pleasant things than chatting with me if the UI and/or SDK methods were more intuitive:) If you keep receiving feedback like this maybe it's worth changing something.

Thank you.

cwaldren-ld commented 2 years ago

Thanks again @phoenix2x for submitting this request!

Tracking internally as 156573.