search

open-feature / flagd

A feature flag daemon with a Unix philosophy
https://openfeature.dev
Apache License 2.0
510 stars 61 forks source link

feat: support relative weighting for fractional evaluation #1313

Closed bacherfl closed 3 months ago

bacherfl commented 4 months ago

Closes #1282

This PR adds support for using relative weights instead of percentages that need to add up to 100. The behavior for existing flag configs does not change with this PR, so those will continue to work as they did previously

netlify[bot] commented 4 months ago

Deploy Preview for polite-licorice-3db33c ready!

Name Link
Latest commit 2c5d23ac0138318d53bbc5ad54c56b9db8b5681f
Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/667d974d0bba1c0008b202c9
Deploy Preview https://deploy-preview-1313--polite-licorice-3db33c.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.64%. Comparing base (1c530ab) to head (23d179d). Report is 83 commits behind head on main.

:exclamation: Current head 23d179d differs from pull request most recent head 2c5d23a

Please upload reports for the commit 2c5d23a to get more accurate results.

Files Patch % Lines
core/pkg/evaluator/fractional.go 92.30% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1313 +/- ## ========================================== + Coverage 73.69% 78.64% +4.95% ========================================== Files 32 36 +4 Lines 3140 2810 -330 ========================================== - Hits 2314 2210 -104 + Misses 717 465 -252 - Partials 109 135 +26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

toddbaert commented 4 months ago

Nice @bacherfl this is great! I will review thoroughly next week. I think we can merge this, but I don't want to add schema support for it until we have something like https://github.com/open-feature/flagd/issues/1312 so that we can track the implementation of this across different providers and avoid confusion as to why some flagd and some providers support it and others do not.

I don't think it will be that much work to support https://github.com/open-feature/flagd/issues/1312. I will follow up next week.

toddbaert commented 4 months ago

@bacherfl @Kavindu-Dodan @thisthat see:

https://github.com/open-feature/flagd-schemas/pull/163 https://github.com/open-feature/flagd/pull/1321

With these, we will serve fully versioned schemas for flagd, which we can use to denote features and fixes such as the one in this PR.

toddbaert commented 3 months ago

@bacherfl @Kavindu-Dodan @thisthat see:

open-feature/flagd-schemas#163 #1321

With these, we will serve fully versioned schemas for flagd, which we can use to denote features and fixes such as the one in this PR.

I'm not doing this afterall. I think it's more maintenance than it's worth, and @aepfli took the time to implement this in most other impls, so I think we can just move forward with this now. I will do the release(s) over the next couple days.