-
Notifications
You must be signed in to change notification settings - Fork 171
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: allow Gateways to filter by pre-release #3111
Conversation
core/capabilities.go
Outdated
minVerNoSuffix, _ := minVer.SetPrerelease("") | ||
verNoSuffix, _ := ver.SetPrerelease("") | ||
// If minVersion has no pre-release component, ignore pre-release versions by | ||
// default as in go-livepeer, we define post-release suffixes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victorges I noticed that I typically use punctuation in my comments, whereas it's not common practice here. I'm happy to adjust and remove it if preferred.
This commit gives Gateways the ability to filter by pre-release suffix. When a pre-release suffix is specified in the `OrchMinLivepeerVersion` command line argument the software now also checks the pre-release version suffix on the orchestrator.
This commit updates the CHANGELOG_PENDING.md file.
694d8f5
to
8e406b4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3111 +/- ##
===================================================
+ Coverage 57.40917% 57.42806% +0.01889%
===================================================
Files 92 92
Lines 15771 15778 +7
===================================================
+ Hits 9054 9061 +7
Misses 6113 6113
Partials 604 604
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
orch = NewCapabilities(nil, nil) | ||
bcast = NewCapabilities(nil, nil) | ||
bcast.constraints.minVersion = "0.4.1" | ||
orch.version = "0.4.1-0.21000000000000-06f1f383fb18" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh if our versions are like this and not like ai-video
or some-specific-feature
then the standard semver comparison from Masterminds/semver
might be great
if minVer.Equal(ver) && minVerHasSuffix && !verHasSuffix { | ||
return false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this logic since Masterminds/semver
already implements proper pre-release comparison, considering pre-releases lower than the release version. Their code is pretty straightforwad, can dig into LessThan
implementation below to understand what they do
Closed as this doesn't make sense for the regular transcoding network since we don't use time-based pre-releases. We use commit suffixes (see https://github.com/livepeer/go-livepeer/blob/master/print_version.sh). |
What does this pull request do? Explain your changes. (required)
This pull request gives Gateways the ability to filter by pre-release suffix. When a pre-release suffix is specified in the
OrchMinLivepeerVersion
command line argument the software now also checks the pre-release version suffix on the orchestrator.Specific updates (required)
OrchMinLivepeerVersion
CLI argument.How did you test each of these updates (required)
Ensured tests worked. Did not run an off-chain or on-chain transcoding gateway yet but did do this on the AI side.
Does this pull request close any open issues?
No but this is required to be able to filter versions on the https://github.com/livepeer/go-livepeer/tree/ai-video branch since we use pre-release suffixes to seperate our versions from the main branhc.
Checklist:
make
runs successfully./test.sh
pass