adding missing until query string from config

[inno-ron]

Pull Request Checklist

Is this in reference to an existing issue?

General

• Update Changelog following the conventions laid out on Keep A Changelog

• Update README with any necessary configuration snippets

• Binstubs are created if needed

• RuboCop passes

• Existing tests pass

New Plugins

• Tests

• Add the plugin to the README

• Does it have a complete header as outlined here

Purpose

Known Compatablity Issues

[inno-ron]

#58

[majormoses]

Thanks for your contribution to Sensu plugins! Without people like you submitting PRs we couldn't run the project. I will review it shortly.

[majormoses]

Thanks for finding, triaging, and proposing a fix for this issue!

I am not 100% sure that my suggestion is required but it certainly seems safer to me.

We need a changelog entry like this and a testing artifact

[majormoses]

From what I read here it does not appear to be a quired field and I am pretty sure if someone does not specify that it will evaluate to nil or brick. I think we should probably do something like this:

url = if !config[:until].nil?
        "#{config[:server]}/render?format=json&target=#{formatted_target}&from=#{config[:from]}&until=#{config[:until]}"
      else
        "#{config[:server]}/render?format=json&target=#{formatted_target}&from=#{config[:from]}"
      end

That being said I don't know that from is either we might want to create a helper function that loops through the list of defined variables and adds them to the string. Something along these lines:

def build_render_query(config)
  url = "#{config[:server]}/render?format=json&target=#{formatted_target}"
  if !config[:from].nil?
    url += "&from=#{config[:from]}"
  if !config[:until].nil?
    url += "&until=#{config[:until]}"
  end
  url
end

then you can set url = build_render_query(config) where the current url definition is.

@eheydrick thoughts?

[multani]

@majormoses FWIW: if you specify either the from or until parameters without a value, it still works as expected. See https://github.com/graphite-project/graphite-web/blob/2a897cf15a1ad9beb2f919267ef98ce40271bea6/webapp/graphite/metrics/views.py#L75:L83 for the code. Also tested on a live Graphite instance.

Actually a proper fix would be:

• filter out nil values (although it doesn't appear to be necessary).
• build the actual query string component using URI.encode_www_form() so everything is properly encoded.

@inno-ron Can I help you with this PR?

[majormoses]

closing due to lack of response, to get this through you can either re-open the PR or open a new one.

@multani sounds like you are willing to work on making this happen. Ping me if I can be of any assistance.