-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Always use localhost, not host name #2367
Conversation
@@ -18,6 +18,8 @@ | |||
- [#2338](https://github.com/influxdb/influxdb/pull/2338): Fix panic if tag key isn't double quoted when it should have been | |||
- [#2340](https://github.com/influxdb/influxdb/pull/2340): Fix SHOW DIAGNOSTICS panic if any shard was non-local. | |||
- [#2351](https://github.com/influxdb/influxdb/pull/2351): Fix data race by rlocking shard during diagnostics. | |||
- [#2350](https://github.com/influxdb/influxdb/pull/2350): Issue fix for :influxd -hostname localhost. |
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.
@corylanou -- this is not the usual style we've been following, unless you are also proposing to merge 2350?
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.
#2350 is an issue, not a PR. The usual style is not to add issue URL, but just the PR URL, and say something like "fixes 2350". Unless I am missing something?
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 was told that based on what Jason/Todd talked about last week, we now add each issue that it fixes, along with the PR.
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.
OK. I thought Jason's change was a once-off, and the format was more like:
#1234 description.....fixes 4678, 3789, 23789
where 1234 is a PR, not an issue.
I think the link is wrong though in your change. It works, but it's actually an issue.
+1 on green build. |
b394ba0
to
1d16d25
Compare
1d16d25
to
3a83c6e
Compare
if c.Hostname, _ = os.Hostname(); c.Hostname == "" { | ||
c.Hostname = "localhost" | ||
} | ||
|
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.
wondering why it wouldn't be good to still have
if c.Hostname == "" {
c.Hostname = "localhost"
}
i'm probably not thinking about something.
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.
c.Hostname
is always ""
when you create a new config.
+1 |
Always use localhost, not host name
Add org admin and member specific permissions
Fixes #2350