Write normalized scheme and host to routing.Route fields #2824
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RomanZavodskikh
added
the
major
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
from
655ab25 to
27d13fd
Compare
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
2 times, most recently
from
b62b0d9 to
01bfbd8
Compare
|
👍 |
szuecs
reviewed
Jan 5, 2024
szuecs
reviewed
Jan 5, 2024
szuecs
reviewed
Jan 5, 2024
szuecs
reviewed
Jan 5, 2024
szuecs
reviewed
Jan 5, 2024
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
3 times, most recently
from
1dddb1d to
469d101
Compare
|
👍 |
szuecs
previously requested changes
Jan 5, 2024
|
As discussed in person, can be a separate pr |
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
from
469d101 to
107e9be
Compare
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
from
107e9be to
8de9679
Compare
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
from
8de9679 to
ce07993
Compare
|
👍 |
szuecs
reviewed
Jan 9, 2024
This change is needed for proxy to easily have normalized host and use it in endpointregistry for dynamic host-wide data to be easily fetchable. Signed-off-by: Roman Zavodskikh <roman.zavodskikh@zalando.de>
RomanZavodskikh
force-pushed
the
normalizeSchemeHost
branch
from
ce07993 to
cf99c04
Compare
|
👍 |
2 similar comments
|
👍 |
|
👍 |
MustafaSaber
reviewed
Jan 9, 2024
| } | ||
|
|
||
| // endpoint address cannot contain path, the rest is not case sensitive | ||
| s, h := strings.ToLower(u.Scheme), strings.ToLower(u.Host) |
There was a problem hiding this comment.
minor: I think variable names here could be more descriptive
scheme, host := strings.ToLower(u.Scheme), strings.ToLower(u.Host)
isolatedHost, port, err := net.SplitHostPort(h)I feel like the current variable names is easy to forget and harder to debug later
This was referenced Jan 9, 2024
This was referenced Apr 22, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change is needed for proxy to easily have normalized host and use it in endpointregistry for dynamic host-wide data to be easily fetchable.
This PR should make changes in #2823 easier.