Skip to content

fix: various custom operator conformance fixes#386

Merged
toddbaert merged 7 commits intoopen-feature:mainfrom
open-feature-forking:fix/evaluator-inconsistencies
Apr 30, 2026
Merged

fix: various custom operator conformance fixes#386
toddbaert merged 7 commits intoopen-feature:mainfrom
open-feature-forking:fix/evaluator-inconsistencies

Conversation

@leakonvalinka
Copy link
Copy Markdown
Member

@leakonvalinka leakonvalinka commented Apr 24, 2026

  • support partial versions in sem_ver
  • return null instead of false for non-string inputs for starts_with and ends_with
  • rename internal parse_version to normalize_version

⚠️ wait until flagd/RPC is updated to merge and remove the added exclude-feature tags in RPC

Fixes: #379


Related:

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the test-harness subproject, introduces partial version support in SemVer parsing by padding with zeros, and changes the return value of string comparisons for non-string inputs from False to None. The review feedback identifies a bug in the version padding logic that breaks versions with suffixes, notes that existing tests need updates to reflect the new return types, and suggests maintaining backward compatibility for the renamed parse_version function.

ends_with,
fractional,
parse_version,
normalize_version,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Renaming parse_version to normalize_version is a breaking change for consumers of this module. Given that this file is explicitly intended for backward compatibility (as noted in the header comment), it should continue to export parse_version as an alias.

Suggested change
normalize_version,
normalize_version,
normalize_version as parse_version,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure about this one, is this necessary? I changed it locally but the pre-commit hook reverted it :D

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think it is is ok. this is an internal function, I don't think anyone relied on it. just to be a bit more carful let's add it as a change in the description.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So you mean that we can keep it as it is right now? Sorry, I'm not entirely sure from your answer.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.20%. Comparing base (c8c9ebe) to head (0472a7a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
+ Coverage   96.14%   96.20%   +0.06%     
==========================================
  Files          47       47              
  Lines        1737     1741       +4     
==========================================
+ Hits         1670     1675       +5     
+ Misses         67       66       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

thanks, looks good 🍻

Comment thread tools/openfeature-flagd-core/tests/test_targeting.py Outdated
@leakonvalinka leakonvalinka marked this pull request as ready for review April 27, 2026 08:52
@leakonvalinka leakonvalinka requested review from a team as code owners April 27, 2026 08:52
leakonvalinka and others added 5 commits April 30, 2026 09:06
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert force-pushed the fix/evaluator-inconsistencies branch from 6c4445d to 175d0a2 Compare April 30, 2026 13:06
toddbaert added a commit to open-feature/flagd that referenced this pull request Apr 30, 2026
* support V prefix in semver
* return null on errors in semver, ends_with, and fractional (already
existed on starts_with)
* fix fractional fallback when total weight is zero

Fixes: #1942

---

Related:
* open-feature/java-sdk-contrib#1778
* open-feature/js-sdk-contrib#1519
* open-feature/dotnet-sdk-contrib#635
* open-feature/python-sdk-contrib#386

---------

Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert merged commit c119a77 into open-feature:main Apr 30, 2026
59 checks passed
@toddbaert toddbaert deleted the fix/evaluator-inconsistencies branch April 30, 2026 20:20
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.

Update flagd-testbed to v3.6.2 and fix in-process evaluation edge cases

6 participants