Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenFeature support for array-like flags. #138

Closed
kinyoklion opened this issue Sep 6, 2022 · 7 comments
Closed

OpenFeature support for array-like flags. #138

kinyoklion opened this issue Sep 6, 2022 · 7 comments

Comments

@kinyoklion
Copy link
Member

In the glossary of terms the example for values utilizes arrays to demonstrate the concept: https://github.com/open-feature/spec/blob/main/specification/glossary.md#values

The evaluation specification doesn't explicitly cover array-like flags: https://github.com/open-feature/spec/blob/main/specification/sections/01-flag-evaluation.md#requirement-131

Though the specification of a structure is vague and potentially could include array-like flags. I think it may be good to clarify their support, or lack of support.

@toddbaert
Copy link
Member

Hey @kinyoklion thanks for noticing this.

I think this might be a good place for a brief discussion on returning arrays in the object evaluation... I wonder if it's worth considering returning our Value type in Java and dotnet, which can represent any JSON node (as well as other similar serialization formats). This would allow us to return arrays, though it also implies the object resolvers could return a wider variety of structures.

@kinyoklion
Copy link
Member Author

I think that the Value type is a good representation for a flag value. That would also allow for heterogeneous flags in the case where a vendor supports it (without having to wrap that flag in an object).

@toddbaert
Copy link
Member

I will play around with this option in the Java SDK. I don't think a spec change is required, since spec is intentionally vague here. I think for many SDKs in languages without static typing and with out-of-the-box JSON parsing, this should "just work".

@toddbaert
Copy link
Member

toddbaert commented Sep 8, 2022

I've tested this idea in the java-sdk and the java-flagd provider, and I have to say I think it's a good solution. It works out to be net negative lines of code in the provider and it gives us the flexibility to return arrays as the object return value (not that every provider would have to support doing so). This wouldn't be a spec change, it's just something that some SDKs would need to do (mostly dotnet and Java).

@rgrassian-split TLDR the discussion here is about changing the Java SDK object flag resolver to return a dev.openfeature.javasdk.Value instead of a dev.openfeature.javasdk.Structure. This allows the return of arrays, which some vendors support. I think it would be a fairly easy change to implement in the split provider. We're trying to get to 1.0 release candidates for all the SDKs, so we'd like to get any such breaking changes out of the way ASAP.

cc @kinyoklion @beeme1mr @justinabrahms @james-milligan @agentgonzo

@rgrassian-split
Copy link
Contributor

@toddbaert @kinyoklion using a Value instead of Structure makes sense to me and should be a small change for our Provider.
However I wanted to ask if there's a way we can change the way we create / get from the Value object, right now it has a bunch of constructors that take in different types of innerObjects and a bunch of gets that make it kind of hard to work with. Can we add an Object constructor as well? On our end we have to make a bunch of if statement checks to find the right constructor, and I could see other Providers facing the same issue to where it might be best to have that logic in the Value object itself.
And for fetching, can we add a get innerObject that just returns an Object? Alternatively there, a variable to keep track of the type and a method to fetch that would also work instead. This is to avoid repeatedly calling the different get methods until a non-null value is returned. I doubt this would be a Split specific issue either and other Providers will probably face the same issues

@toddbaert
Copy link
Member

toddbaert commented Sep 8, 2022

Can we add an Object constructor as well? ... And for fetching, can we add a get innerObject that just returns an Object?

I see no issue with this additional flexibility, though I think I'd prefer to call it asObject(). Feel free to open a PR or I will do so when I action the Value/Structure change, unless anyone else has an objection.

@toddbaert
Copy link
Member

toddbaert commented Sep 13, 2022

@kinyoklion - closing this since I believe it's fully supported now via: open-feature/dotnet-sdk#57 and open-feature/java-sdk#65.

Please re-open if I'm wrong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants