Allow configuration by an URL

[carlhoerberg]

Allows you to configure the Influx client with an URL, eg:

InfluxDB::Client.new url: "tls://user:pass@myhost/mydb"

[dmke]

I like the idea, because it enables the configuration to be a single environment variable.

However, I find the merge_url method a bit too magical (for the lack of a better word). It mutates its argument, which I don't expect from Ruby methods (it's technically not an error, but unexpected).

How about something like this?

def initialize(opts = {})
  if opts.key?(:url)
    opts.merge! options_from_url URL.parse opts[:url]
  end
  # ...
end

def options_from_url(url)
  {}.tap do |opts|
    opts[:host] = url.host
    opts[:port] = url.port if url.port
    # etc.
  end
end

This makes #options_from_url easier to test as well (speaking of... do you mind adding a context block to spec/influxdb/config_spec.rb?)

[dmke]

On another note, InfluxDB only speaks HTTP(S) and UDP, so I'd prefer the URI scheme to reflect that (i.e. %w[http https upd].include?(uri.scheme)).

Another addition could be a mapping of the client options to URI query parameters, for example:

cli = InfluxDB::Client.new url: "https://localhost:8083/foodb?denormalize=true"
# instead of
# InfluxDB::Client.new url: "https://localhost:8083/foodb", denormalize: true

thus enabling all configuration to be made with a single environment variable.

This might be out of scope for this PR, though. Feel free to leave this part out :-)

[carlhoerberg]

No, thank you for the feedback! Will work on it next week.

On May 28, 2017 22:37, "Dominik Menke" notifications@github.com wrote:

On another note, InfluxDB only speaks HTTP(S) and UDP, so I'd prefer the URI scheme to reflect that (i.e. %w[http https upd].include?(uri.scheme)).

Another addition could be a mapping of the client options to URI query parameters, for example:

cli = InfluxDB::Client.new url: "https://localhost:8083/foodb?denormalize=true"# instead of# InfluxDB::Client.new url: "https://localhost:8083/foodb", denormalize: true

thus enabling all configuration to be made with a single environment variable.

This might be out of scope for this PR, though. Feel free to leave this part out :-)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/influxdata/influxdb-ruby/pull/188#issuecomment-304538637, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK_Ti2amqIjWAtV8aFxEHSpIzLyKbyCks5r-dsWgaJpZM4NhTbv .

[dmke]

It has been a while. Are you still working on this?

[carlhoerberg]

I can, sorry, other things came in between ..

On Jun 28, 2017 08:09, "Dominik Menke" notifications@github.com wrote:

It has been a while. Are you still working on this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/influxdata/influxdb-ruby/pull/188#issuecomment-311565683, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK_Tqet0SkV99cPyJTbyPfFl3jOv0phks5sIe4jgaJpZM4NhTbv .

[dmke]

thus enabling all configuration to be made with a single environment variable

I have implemented this now, and... well... Let's say the Config class needed some major refactoring :-)

Closing this now. A release of 0.4.0 follows tomorrow, I think.