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

Fixes typos in sample config file #2647

Merged
merged 4 commits into from
May 28, 2015
Merged

Fixes typos in sample config file #2647

merged 4 commits into from
May 28, 2015

Conversation

claws
Copy link
Contributor

@claws claws commented May 24, 2015

I have been working on installing the latest version of Influx (0.9) onto a system that is not connected to the internet. This involves building it from source, on 64-bit Ubuntu 14.04, and sourcing any dependencies without resorting to go get.

Like most users I start with the sample config file and tweak it to suit my scenario. While looking at various details I noticed that the etc/config.sample.toml contains fields which do not match the code. This means that if I uncomment these fields their values will not be used which is not what a normal user would expect.

The code that parses the Collectd section of the configuration file tries to read a field called bind-address and if it can't be found then the fallback defaultBindAddr is used. Similarly, the Graphite section parsing code does the same thing.

In the Influx sample configuration file etc/config.sample.toml the address fields for both Collectd and Graphite are labelled address. A normal user would expect to be able to uncomment these and have them work. However, because of the mismatch already mentioned, this would not happen.

Collectd:

etc/config.sample.toml:

#address = "0.0.0.0" # If not set, is actually set to bind-address.

cmd/influxd/config.go:

BindAddress string `toml:"bind-address"`

Graphite:

etc/config.sample.toml:

#address = "0.0.0.0" # If not set, is actually set to bind-address.

cmd/influxd/config.go:

BindAddress string `toml:"bind-address"`

The config_test.go file contains the correct bind-address fields. So, it just looks like the content in the etc/config.sample.toml has become inconsistent with the code.

This pull request just fixes the fields in the etc/config.sample.toml so they are consistent with the code. It also adds a space after some of the comment characters so the file looks consistent.

A better long term solution might be to generate the default configuration file from the application. This would hopefully avoid the issue of inconsistency between the sample configuration file and what the code expects.

From a consistency perspective it would be good if the address field under the opentsdb section also used bind-address like the Collectd and Graphite sections. This is likely a job for a separate pull request.

@corylanou
Copy link
Contributor

+1. Can you also update CHANGELOG.md, and if you haven't already, please sign the CLA

@toddboom
Copy link
Contributor

+1 and the same things @corylanou suggested

@claws
Copy link
Contributor Author

claws commented May 28, 2015

Signed CLA, updated CHANGELOG.md.

@toddboom toddboom merged commit ac76e5c into influxdata:master May 28, 2015
@toddboom toddboom removed the review label May 28, 2015
@claws claws deleted the fix_sample_config_typos branch May 29, 2015 23:52
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.

None yet

3 participants