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

[do not merge] Set endpoint Priority #4454

Closed
wants to merge 2 commits into from

Conversation

akerouanton
Copy link
Member

Related to:

- What I did

The new Priority field added to EndpointSettings by moby/moby#46036 is used when multiple --network options are passed to docker create / docker run.

This first commit manually edit github.com/docker/docker/api/types/network. A proper revendoring will be needed once moby/moby#46036 is merged.

- How to verify it

CI is green

- A picture of a cute animal (not mandatory but encouraged)

…work

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
@akerouanton
Copy link
Member Author

Moby PR moby/moby#46036 has been closed in favor of moby/moby#48936. Moreover, after discussing this internally, we realize that setting the new GwPriority based on the order of --network flags would be a breaking change, so better not implement that. We might reconsider this decision in the future based on user feedback. For now, let me close this PR.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.40%. Comparing base (dbd53f0) to head (c1ba519).
Report is 1609 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4454      +/-   ##
==========================================
- Coverage   59.41%   59.40%   -0.02%     
==========================================
  Files         288      288              
  Lines       24776    24784       +8     
==========================================
+ Hits        14720    14722       +2     
- Misses       9171     9175       +4     
- Partials      885      887       +2     

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants