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

NetworksByPriority: ensure the sort order is stable #580

Merged

Conversation

akerouanton
Copy link
Contributor

@akerouanton akerouanton commented Feb 16, 2024

Related to:

NetworksByPriority calls sort.Slice() to sort the map of networks attached to a given service by their respective priority. The Priority property being non mandatory, it defaults to 0 when unspecified.

The godoc for sort.Slice() specifies:

The sort is not guaranteed to be stable: equal elements may be reversed from their original order. For a stable sort, use SliceStable.

SliceStable() won't work either, since NetworksByPriority has to transform the map of networks into a list before calling sort.Slice() -- Go makes no guarantee about the order of items in a map.

The only way to guarantee a stable order is to fall back to lexicographic sorting when two networks have the same priority. It seems to be the best option as the compose-spec doesn't specify how multiple network attachment referencing the same network should be handled.

A regression test has been added.

NetworksByPriority calls sort.Slice() to sort the map of networks
attached to a given service by their respective priority. The
Priority property being non mandatory, it defaults to 0 when
unspecified.

The godoc for sort.Slice() specifies:

> The sort is not guaranteed to be stable: equal elements may be
> reversed from their original order. For a stable sort, use
> SliceStable.

SliceStable() won't work either, since NetworksByPriority has to
transform the map of networks into a list before calling
sort.Slice() -- Go makes no guarantee about the order of items in
a map.

The only way to guarantee a stable order is to fall back to
lexicographic sorting when two networks have the same priority. It
seems to be the best option as the compose-spec doesn't specify how
multiple network attachment referencing the same network should be
handled.

A regression test has been added.

Signed-off-by: Albin Kerouanton <[email protected]>
@akerouanton akerouanton requested a review from ndeloof as a code owner February 16, 2024 23:54
@ndeloof ndeloof merged commit 758459d into compose-spec:main Feb 17, 2024
8 checks passed
@akerouanton akerouanton deleted the fix-NetworksByPriority-stable-order branch February 19, 2024 07:27
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.

2 participants