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.

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

introduce gw_priority #731

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Jan 22, 2025

@ndeloof
Copy link
Collaborator Author

ndeloof commented Jan 22, 2025

cc @robmry

@ndeloof ndeloof requested a review from glours January 22, 2025 16:39
Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

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

Oh, thank you! I was about to have a look at this one ... but only got as far as realising I should've updated compose-spec's schema/compose-spec.json in compose-spec/compose-spec#552, I'll raise a new PR for that. (Also for EnableIPv4 in compose-spec/compose-spec#551, but that's not merged yet.)

LGTM - although perhaps it's worth adding to full-example.yml / full-struct_test.go?

@ndeloof
Copy link
Collaborator Author

ndeloof commented Jan 22, 2025

@robmry we use a bot to sync schema changes made in compose-go into the spec
About full-struct_test.go, we avoid using this huge beast for new feature, and prefer dedicated tests per feature - but for such a trivial thing as a plain yaml mapping we don't require one

@robmry
Copy link
Contributor

robmry commented Jan 22, 2025

Ah, got it! Thank you.

Signed-off-by: Nicolas De Loof <[email protected]>
@ndeloof
Copy link
Collaborator Author

ndeloof commented Jan 24, 2025

adding support for this in docker/compose is a bit challenging as we can't get API 1.48 from any tag, and depending on master bring dependency updates that can create troubles.

@robmry
Copy link
Contributor

robmry commented Jan 24, 2025

adding support for this in docker/compose is a bit challenging as we can't get API 1.48 from any tag, and depending on master bring dependency updates that can create troubles.

I expect we'll create a 28.x branch from master, with a release candidate label, in the next week or two ... but it'd be good to find the problems sooner rather than later. Would it be possible to give it an early run in a PR that's not mergeable until the tag's available?

@ndeloof
Copy link
Collaborator Author

ndeloof commented Jan 24, 2025

@robmry I've tried to setup a test branch to build with docker/docker@master and check things work as expected, but hen I get some weird dependency downgrades that break everything:

$ go get -u github.com/docker/docker@master
go: downloading github.com/docker/docker v27.0.2-0.20250123194108-433e9a50ce7b+incompatible
go: downgraded github.com/docker/buildx v0.20.1 => v0.15.1
go: downgraded github.com/docker/docker v27.5.1+incompatible => v27.0.2-0.20250123194108-433e9a50ce7b+incompatible
go: downgraded github.com/moby/buildkit v0.19.0 => v0.14.1

I guess this is because last know tag on docker/docker@master is v27.0.2 then go mod selects the latest buildx tag to match this version.

@robmry
Copy link
Contributor

robmry commented Jan 24, 2025

Oh, I see - thank you.

@thaJeztah
Copy link
Member

Oh! go modules pseudo-version-halucination striking again!

So the official recommendation from the Go folks is to just willy-nilly tag some commits as v-<next-version>-alpha.0, but that makes NO sense at all when taking SemVer into account (what’s the next version gonna be? you won’t know until you do the release if you must update “major”, “minor” or “patch”).

In the end, go module should not hallucinate versions for anything untagged, and consider it “you want YOLO? here you go!”

But the ugly workaround to achieve that without them changing, is to add a replace;

replace github.com/docker/docker  => github.com/docker/docker master

Or using go mod edit;

go mod edit -replace=github.com/docker/docker=github.com/docker/docker@master

After you added the replace, you can run go mod tidy to make it resolve the actual commit to use;

go mod tidy

And go.mod should then show the correct version;

git diff
diff --git a/go.mod b/go.mod
index c5d2118..6e8bffc 100644
--- a/go.mod
+++ b/go.mod
@@ -26,3 +26,5 @@ require (
        github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
        golang.org/x/sys v0.1.0 // indirect
 )
+
+replace github.com/docker/docker => github.com/docker/docker v27.0.2-0.20250123194108-433e9a50ce7b+incompatible

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.

3 participants