-
Notifications
You must be signed in to change notification settings - Fork 1
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
[NH-81807] Use a default port (443) for collector address #102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @swi-jared!
@@ -86,7 +89,14 @@ func ToServiceKey(s string) string { | |||
|
|||
// IsValidHost verifies if the host is in a valid format | |||
func IsValidHost(host string) bool { | |||
// TODO | |||
if _, _, err := net.SplitHostPort(host); err != nil { | |||
if strings.Contains(err.Error(), "missing port in address") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend to check the negation so we don't have an empty scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean by check the negation? Also, what empty scope?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my bad. I read it wrongly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -86,7 +89,14 @@ func ToServiceKey(s string) string { | |||
|
|||
// IsValidHost verifies if the host is in a valid format | |||
func IsValidHost(host string) bool { | |||
// TODO | |||
if _, _, err := net.SplitHostPort(host); err != nil { | |||
if strings.Contains(err.Error(), "missing port in address") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my bad. I read it wrongly.
No description provided.