flagsmith icon indicating copy to clipboard operation
flagsmith copied to clipboard

Introduce the SDK Evaluation Context object

Open khvn26 opened this issue 1 year ago • 4 comments

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:

  • Implement it in the Golang SDK as part of #4278, and evolve the existing interfaces for the rest of the SDKs.
  • Implement it for all SDKs as part of #4278.

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 avatar Jul 29 '24 16:07 khvn26

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 avatar Jul 29 '24 16:07 matthewelwell

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.

matthewelwell avatar Jul 29 '24 17:07 matthewelwell

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

zachaysan avatar Jul 29 '24 19:07 zachaysan

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

matthewelwell avatar Jul 30 '24 09:07 matthewelwell