Ignore ELASTICSEARCH_URL when config set?

[graphaelli]

Per

go-elasticsearch/elasticsearch.go

Lines 86 to 100 in cb4ea94

	// It's an error to set both cfg.Addresses and the ELASTICSEARCH_URL
	// environment variable.
	//
	func NewClient(cfg Config) (*Client, error) {
	var addrs []string
	envAddrs := addrsFromEnvironment()
	if len(envAddrs) > 0 && len(cfg.Addresses) > 0 {
	return nil, errors.New("cannot create client: both ELASTICSEARCH_URL and Addresses are set")
	}
	if len(envAddrs) > 0 && cfg.CloudID != "" {
	return nil, errors.New("cannot create client: both ELASTICSEARCH_URL and CloudID are set")
	}

, library users are not permitted to set both ELASTICSEARCH_URL in the environment and pass in addresses - I'd like to reconsider this limitation, perhaps some background on why would help? I don't see the same restriction in the python client.

APM Server recently started using go-elasticsearch and coincidentally someone introduced ELASTICSEARCH_URL into the test environment, causing tests fail as the es client could not be created. This was surprising since APM Server always creates a client with config.Addresses.

[karmi]

I think the motivation was to prevent surprises about what happens when both are set: does one take precedence over the other? Do they get merged? (Eg., in your case, if the environment variable gets set suddenly in a test environment, should it be picked up by the client?)

I'm fine with removing the restriction, but we need to figure out a clear solution to the case when both are set. Intuitively, I'd say that when config.Addresses are set, the environment variable would be ignored.

[graphaelli]

Agreed, I'd expect code to take precedence over environment variables.

[karmi]

I've merged the 86a626c commit into master, 6.x and 7.x branches. It will be part of the next release.