Conversation
dosubot
bot
added
the
size:XL
✅ Deploy Preview for polite-licorice-3db33c canceled.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the fractional evaluation feature by increasing its precision and adding support for nested JSONLogic, providing more flexibility in flag configurations. It also includes dependency updates and integration test improvements to ensure the reliability of the changes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The fractional evaluator was updated to support boolean, numeric, and nil variants, and also nested JSON logic. The totalWeight and variant weight are now int32 instead of int to support larger weight values. The hashing of the value to distribute is now done in the Evaluate function rather than in the distributeValue function. The test suite was updated to use constants for email addresses and other string literals to improve readability and maintainability. The test suite was updated to add tests for nested JSON logic.
|
|
||
| func (fe *Fractional) Evaluate(values, data any) any { | ||
| valueToDistribute, feDistributions, err := parseFractionalEvaluationData(values, data) | ||
| valueToDistribute, feDistributions, err := parseFractionalEvaluationData(values, data, fe.Logger) |
There was a problem hiding this comment.
The fe.Logger is passed as an argument to parseFractionalEvaluationData. This is good for testability and dependency injection.
| hashValue := uint32(murmur3.StringSum32(valueToDistribute)) | ||
| return distributeValue(hashValue, feDistributions) |
There was a problem hiding this comment.
The hashing of the valueToDistribute is now done in the Evaluate function rather than in the distributeValue function. This centralizes the hashing logic and makes the distributeValue function more focused on its core task.
| return "" | ||
| } | ||
|
|
||
| bucket := (uint64(hashValue) * uint64(feDistribution.totalWeight)) >> 32 |
There was a problem hiding this comment.
This calculation is the core of the high-precision fractional bucketing. It uses integer arithmetic to map the hash value to a bucket within the range [0, totalWeight).
There was a problem hiding this comment.
Weird that gemini seems to just be making a lot of these observational comment lately.
fractional Argument Evaluation in flagd provider
open-feature/java-sdk-contrib#1737
toddbaert
requested review from
MarkoMlakar,
aepfli,
alexandraoberaigner,
beeme1mr,
leakonvalinka and
marcozabel
toddbaert
force-pushed
the
feat/fractional-enhancements
branch
from
4f7cc84 to
f4ea487
Compare
* fractional now supports up to max-int32 total weight, and 1/max-int32 resolution * fractional now supports computed (nested JSONLogic) variants and weights Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
feat/fractional-enhancements
branch
from
f4ea487 to
1e7175b
Compare
toddbaert
changed the title
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
force-pushed
the
feat/fractional-enhancements
branch
from
94a3905 to
8d04a4f
Compare
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
|
toddbaert
changed the title
2 participants
High-precision and nested fractional
This is a coordinated feature across flagd and all flagd providers.
fractionalnow supports up to max-int32 total weight and 1/max-int32 resolution as described in the high-precision fractional bucketing ADRfractionalnow supports computed (nested JSONLogic) variants and weights as described in the fractional ADRRelated PRs