Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: allow Gateways to filter by pre-release #3111

Closed
wants to merge 2 commits into from
Closed

feat: allow Gateways to filter by pre-release #3111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b94a26
feat: allow Gateways to filter by pre-release
rickstaa Jul 31, 2024
8e406b4
chore: update pending changelog
rickstaa Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG_PENDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased Changes

- [#3111](https://github.com/livepeer/go-livepeer/pull/3111) feat: allow Gateways to filter by pre-release

## vX.X

### Breaking Changes 🚨🚨
Expand Down
19 changes: 15 additions & 4 deletions core/capabilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,22 @@ func (bcast *Capabilities) LivepeerVersionCompatibleWith(orch *net.Capabilities)
return false
}

// Ignore prerelease versions as in go-livepeer we actually define post-release suffixes
minVerNoSuffix, _ := minVer.SetPrerelease("")
verNoSuffix, _ := ver.SetPrerelease("")
// If minVersion has no pre-release component, ignore pre-release versions.
// If versions match without pre-release, but only minVersion has a suffix, return false.
// Needed because we use post-release suffixes in go-livepeer.
minVerHasSuffix := minVer.Prerelease() != ""
verHasSuffix := ver.Prerelease() != ""
if minVer.Prerelease() == "" || ver.Prerelease() == "" {
minVerNoSuffix, _ := minVer.SetPrerelease("")
verNoSuffix, _ := ver.SetPrerelease("")
minVer = &minVerNoSuffix
ver = &verNoSuffix
}
if minVer.Equal(ver) && minVerHasSuffix && !verHasSuffix {
return false
}
Comment on lines +355 to +357
Copy link
Member

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


return !verNoSuffix.LessThan(&minVerNoSuffix)
return !ver.LessThan(minVer)
}

func (bcast *Capabilities) CompatibleWith(orch *net.Capabilities) bool {
Expand Down
54 changes: 53 additions & 1 deletion core/capabilities_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,40 @@ func TestCapability_CompatibleWithNetCap(t *testing.T) {
orch.version = "0.4.0"
assert.False(bcast.CompatibleWith(orch.ToNetCapabilities()))

// broadcaster is not compatible with orchestrator - the same version
// broadcaster is compatible with orchestrator - the same version
orch = NewCapabilities(nil, nil)
bcast = NewCapabilities(nil, nil)
bcast.constraints.minVersion = "0.4.1"
orch.version = "0.4.1"
assert.True(bcast.CompatibleWith(orch.ToNetCapabilities()))

// broadcaster is compatible with orchestrator - no pre-release min version suffix
orch = NewCapabilities(nil, nil)
bcast = NewCapabilities(nil, nil)
bcast.constraints.minVersion = "0.4.1"
orch.version = "0.4.1-0.21000000000000-06f1f383fb18"
Copy link
Member

@victorges victorges Aug 1, 2024 •
edited
Loading

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

assert.True(bcast.CompatibleWith(orch.ToNetCapabilities()))

// broadcaster is not compatible with orchestrator - no pre-release O's version
orch = NewCapabilities(nil, nil)
bcast = NewCapabilities(nil, nil)
bcast.constraints.minVersion = "0.4.1-0.20000000000000-06f1f383fb18"
orch.version = "0.4.1"
assert.False(bcast.CompatibleWith(orch.ToNetCapabilities()))

// broadcaster is not compatible with orchestrator pre-release - old O's version
orch = NewCapabilities(nil, nil)
bcast = NewCapabilities(nil, nil)
bcast.constraints.minVersion = "0.4.1-0.21000000000000-06f1f383fb18"
orch.version = "0.4.1-0.20000000000000-06f1f383fb18"
assert.False(bcast.CompatibleWith(orch.ToNetCapabilities()))

// broadcaster is compatible with orchestrator pre-release - higher O's version
orch = NewCapabilities(nil, nil)
bcast = NewCapabilities(nil, nil)
bcast.constraints.minVersion = "0.4.1-0.20000000000000-06f1f383fb18"
orch.version = "0.4.1-0.21000000000000-06f1f383fb18"
assert.True(bcast.CompatibleWith(orch.ToNetCapabilities()))
}

func TestCapability_RoundTrip_Net(t *testing.T) {
Expand Down Expand Up @@ -568,6 +596,30 @@ func TestLiveeerVersionCompatibleWith(t *testing.T) {
transcoderVersion: "nonparsablesemversion",
expected: false,
},
{
name: "broadcaster required version has no pre-release",
broadcasterMinVersion: "0.4.1",
transcoderVersion: "0.4.1-0.21000000000000-06f1f383fb18",
expected: true,
},
{
name: "transcoder version has no pre-release",
broadcasterMinVersion: "0.4.1-0.21000000000000-06f1f383fb18",
transcoderVersion: "0.4.1",
expected: false,
},
{
name: "broadcaster required pre-release version is higher than transcoder pre-release version",
broadcasterMinVersion: "0.4.1-0.21000000000000-06f1f383fb18",
transcoderVersion: "0.4.1-0.20000000000000-06f1f383fb18",
expected: false,
},
{
name: "broadcaster required pre-release version is lower than transcoder pre-release version",
broadcasterMinVersion: "0.4.1-0.20000000000000-06f1f383fb18",
transcoderVersion: "0.4.1-0.21000000000000-06f1f383fb18",
expected: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading