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

feat: Introduce the SDK Evaluation Context schema #4414

Merged
merged 8 commits into from
Oct 3, 2024

Conversation

khvn26
Copy link
Member

@khvn26 khvn26 commented Jul 29, 2024

Thanks for submitting a PR! Please check the boxes below:

Contributes to #4413.

  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

The following is a proposed Flagsmith Evaluation Context schema JSON Schema definition.

Notes

  • IdentityEvaluationContext.traits is a mapping of trait keys to a TraitEvaluationContext | null union. In case a null is provided, we assume intent to delete the previously persisted trait.
  • TraitEvaluationContext.value can only be a string now.
  • We now have a TraitEvaluationContext.type enum field designating the trait value type. For now it will support an SDK-side mapping layer. It's expected that the v2 SDK API should support it in the future.

How did you test this code?

This is a documentation change, but I'm verifying the schema against a validator.

@khvn26 khvn26 requested review from a team and novakzaballa July 29, 2024 17:11
Copy link

vercel bot commented Jul 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Aug 6, 2024 9:16pm
flagsmith-frontend-preview ⬜️ Ignored (Inspect) Visit Preview Aug 6, 2024 9:16pm
flagsmith-frontend-staging ⬜️ Ignored (Inspect) Visit Preview Aug 6, 2024 9:16pm

@github-actions github-actions bot added the feature New feature or request label Jul 29, 2024
Copy link
Contributor

github-actions bot commented Jul 29, 2024

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-4414 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-4414 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-4414 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-4414 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-4414 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4414 Finished ✅ Results

Copy link
Contributor

github-actions bot commented Jul 29, 2024

Uffizzi Ephemeral Environment deployment-54754

☁️ https://app.uffizzi.com/github.com/Flagsmith/flagsmith/pull/4414

📄 View Application Logs etc.

What is Uffizzi? Learn more!

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Jul 29, 2024
@khvn26 khvn26 marked this pull request as ready for review July 31, 2024 09:04
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Jul 31, 2024
@novakzaballa
Copy link
Contributor

@khvn26 I love the idea of a schema.
I would like to see examples of how this schema would be used. While I think the current schema is complete and clear, I think we could simplify the structure of some of the contexts, but that would be clearer if we could have some implementation examples of the schema.

@khvn26
Copy link
Member Author

khvn26 commented Jul 31, 2024

@novakzaballa One thing I could do to demonstrate is to go ahead and add it for JS and Golang SDK transient ids/traits PRs. You'll be able to review the schema with those implementations in consideration.

@novakzaballa
Copy link
Contributor

@novakzaballa One thing I could do to demonstrate is to go ahead and add it for JS and Golang SDK transient ids/traits PRs. You'll be able to review the schema with those implementations in consideration.

Let's do that @khvn26 . Some examples I generated look good to me and we can always optimize if we see an opportunity while we move forward. I'll approve the PR. Nice job!

Copy link
Contributor

@novakzaballa novakzaballa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 2, 2024
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 2, 2024
@khvn26 khvn26 force-pushed the feat/evaluation-context-schema branch from 793dd11 to 741e68f Compare August 2, 2024 15:37
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 2, 2024
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 2, 2024
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 5, 2024
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 6, 2024
@khvn26 khvn26 added this pull request to the merge queue Oct 3, 2024
Merged via the queue into main with commit d6c6004 Oct 3, 2024
31 checks passed
@khvn26 khvn26 deleted the feat/evaluation-context-schema branch October 3, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants