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: move json logic operator registration to resolver #1291

Conversation

Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Apr 18, 2024

This PR

Attempts to isolate Resolver creation and related logic into a single component.

Previously, custom Json logic operator registration was done at flagd using WithEvaluator option . But this adds confusion when Resolver needs to be reused in another component. Further the option was merely adding the custom operator to json logic through its global methods and did not perform anything on the evaluator . Given that Resolver is responsible for flag resolving (core logic of the flag evaluations), I have deprecated WithEvaluator option and added custom json logic registration as part of the resolver constructor.

I have not removed JSONEvaluatorOption option so we have flexibility to support any future option (ex:-option to have a customized IResolver implemetnation )

@Kavindu-Dodan Kavindu-Dodan requested a review from a team as a code owner April 18, 2024 17:02
Copy link

netlify bot commented Apr 18, 2024

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit 1c51074
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/662155b9cf66830007cf169f

Copy link

codecov bot commented Apr 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.41%. Comparing base (1c530ab) to head (5d010e2).
Report is 51 commits behind head on main.

❗ Current head 5d010e2 differs from pull request most recent head 1c51074. Consider uploading reports for the commit 1c51074 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1291      +/-   ##
==========================================
+ Coverage   73.69%   77.41%   +3.72%     
==========================================
  Files          32       20      -12     
  Lines        3140     1612    -1528     
==========================================
- Hits         2314     1248    -1066     
+ Misses        717      282     -435     
+ Partials      109       82      -27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan Kavindu-Dodan force-pushed the feat/bind-custom-json-logic-operators-to-resolver branch from 5d010e2 to 1c51074 Compare April 18, 2024 17:17
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Ya this makes sense.

@toddbaert toddbaert merged commit b473457 into open-feature:main Apr 18, 2024
13 checks passed
@github-actions github-actions bot mentioned this pull request Apr 18, 2024
Kavindu-Dodan pushed a commit that referenced this pull request Apr 19, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.10.1</summary>

##
[0.10.1](flagd/v0.10.0...flagd/v0.10.1)
(2024-04-19)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.9.0
([#1281](#1281))
([3cfb052](3cfb052))


### ✨ New Features

* move json logic operator registration to resolver
([#1291](#1291))
([b473457](b473457))
</details>

<details><summary>flagd-proxy: 0.6.1</summary>

##
[0.6.1](flagd-proxy/v0.6.0...flagd-proxy/v0.6.1)
(2024-04-19)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.9.0
([#1281](#1281))
([3cfb052](3cfb052))
</details>

<details><summary>core: 0.9.1</summary>

##
[0.9.1](core/v0.9.0...core/v0.9.1)
(2024-04-19)


### 🐛 Bug Fixes

* missing/nil custom variables in fractional operator
([#1295](#1295))
([418c5cd](418c5cd))


### ✨ New Features

* move json logic operator registration to resolver
([#1291](#1291))
([b473457](b473457))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

None yet

2 participants