feat: support semantic version conditions - #365
Conversation
49f2b5f to
797e813
Compare
greghuels
changed the title
797e813 to
b5d8726
Compare
greghuels
changed the title
| allocation.splits.every((split) => Array.isArray(split.shards)) && | ||
| (allocation.rules === undefined || | ||
| (Array.isArray(allocation.rules) && allocation.rules.every((rule) => isValidRule(rule)))) | ||
| Array.isArray(allocation.rules) && allocation.rules.some((rule) => hasInvalidSemverComparand(rule)) |
There was a problem hiding this comment.
minor: shall this be moved to isValidRule?
| } | ||
|
|
||
| const value = version.slice(start, end) | ||
| if (value.length > MAX_UINT64.length || (value.length === MAX_UINT64.length && value > MAX_UINT64)) { |
There was a problem hiding this comment.
minor: why do we artificially limit the max value?
There was a problem hiding this comment.
For parity with implementation in other dd-trace libraries. It shouldn't matter too much either way, since this won't be a real-world use case.
| * Core identifiers are limited to uint64; numeric prerelease identifiers may | ||
| * be arbitrarily large. | ||
| */ | ||
| export function parseSemver(version: unknown): ParsedSemver | null { |
There was a problem hiding this comment.
nitpick: the current implementation seems to be verbose and likely slow'ish
I have a feeling that a regex could be faster and shorter. Another option is to split on . and - and validate components afterwards
There was a problem hiding this comment.
I had an LLM run through some benchmarks on the 2 implementations you suggested along with the current implementation. It looks like the current implementation is still the fastest.
2 participants
Summary
@datadog/flagging-core.SEMVER_EQ,SEMVER_NEQ,SEMVER_LT,SEMVER_LTE,SEMVER_GT, andSEMVER_GTE.DatadogNodeServerProvider.ffe-system-test-datatoaaa97e4, which has the SEMVER system tests.Why uint64-safe string comparison
The evaluator keeps SemVer core components as decimal strings instead of converting them to JavaScript
Numbervalues.uint64-bounded core components, including values aboveNumber.MAX_SAFE_INTEGER, so narrowing JavaScript toNumber.MAX_SAFE_INTEGERwould create cross-SDK differences.Numberparsing on every evaluation and is faster than converting the component strings to numbers at comparison time. It also avoids precision loss for large valid versions.