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

sort Outbound selector output #2914

Merged
merged 4 commits into from
Jan 10, 2024
Merged

Conversation

hossinasaadi
Copy link
Contributor

@hossinasaadi hossinasaadi commented Jan 8, 2024

as mentioned #2898 (comment)
outbound selector tags output may not run by order in :

for tag := range m.taggedHandler {

as mentions in golang ref
See https://golang.org/ref/spec#For_statements

The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next. If a map entry that has not yet been reached is removed during iteration, the corresponding iteration value will not be produced. If a map entry is created during iteration, that entry may be produced during the iteration or may be skipped. The choice may vary for each entry created and from one iteration to the next. If the map is nil, the number of iterations is 0.

@yomnxkcs
Copy link

yomnxkcs commented Jan 8, 2024

If anyone misconfigured like below, each tag will be added twice.

{
  "routing": {
    "balancers": [
      {
        "selector": [
          "pro",
          "proxy"
        ]
      }
    ]
  }
}

Maybe we should add a break after append().
Besides that, all looks good.

@hossinasaadi
Copy link
Contributor Author

hossinasaadi commented Jan 8, 2024

break

that was a good point. 👍

@yuhan6665 yuhan6665 merged commit 81f9f56 into XTLS:main Jan 10, 2024
34 checks passed
@yuhan6665
Copy link
Member

Looks good! Thanks all

@hossinasaadi hossinasaadi deleted the OutboundSelector branch January 11, 2024 00:37
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