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

Introduce the SDK Evaluation Context object #4413

Open
khvn26 opened this issue Jul 29, 2024 · 4 comments
Open

Introduce the SDK Evaluation Context object #4413

khvn26 opened this issue Jul 29, 2024 · 4 comments
Labels
api Issue related to the REST API sdk Issues related to the SDKs

Comments

@khvn26
Copy link
Member

khvn26 commented Jul 29, 2024

Abstract

While implementing #4278, a discussion has occurred about how we should approach SDK changes in the future.

The consensus is we should take after OpenFeature and implement a common evaluation context object that a single getFlags interface should accept as its only input.

I'm creating this issue as a hub for further implementation discussion and planning.

Schema proposal

You can observe and review the proposed schema in #4414.

Implementation plan

For implementation phase 1, We should settle on one of the following:

For phase 2, we should consider a single /api/v2/flags API with a schema identical to the Evaluation Context schema. Should we go forward with this, it'll allow us to fully code generate remote evaluation parts of the SDKs!

@khvn26 khvn26 added sdk Issues related to the SDKs api Issue related to the REST API labels Jul 29, 2024
@matthewelwell
Copy link
Contributor

My preference for the implementation is (1) as I am keen to get transient traits out there ASAP. It also gives us some time to perfect the implementation and, if we decide it's not quite right, we would only need a breaking change in a single SDK.

The only concern here would be that the documentation becomes slightly more difficult, but I think we can get around that.

@matthewelwell
Copy link
Contributor

A few comments on the schema:

  1. I see that Traits is an object. Should this be a list?
  2. I see that environment is required, since the evaluation context (as I understand it), is provided when requesting the flags, shouldn't the environment default to what was provided when initialising the client, or are we removing that entirely from init?
  3. Let's make sure that we're consistent with our casing. Maybe I'm missing a pattern, but it's odd to me that e.g. environment, feature, etc. are lower case, but Traits is upper case.

@zachaysan
Copy link
Contributor

I mirror @matthewelwell's comments on the approach. But other than that the overall approach seems sensible to me.

@matthewelwell
Copy link
Contributor

A few comments on the schema:

  1. I see that Traits is an object. Should this be a list?
  2. I see that environment is required, since the evaluation context (as I understand it), is provided when requesting the flags, shouldn't the environment default to what was provided when initialising the client, or are we removing that entirely from init?
  3. Let's make sure that we're consistent with our casing. Maybe I'm missing a pattern, but it's odd to me that e.g. environment, feature, etc. are lower case, but Traits is upper case.

Added the one outstanding comment to the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API sdk Issues related to the SDKs
Projects
None yet
Development

No branches or pull requests

3 participants