Extract endpoints into config file

[willzoltan]

There are two places in the SDK which reference the Cronitor API*:

• Cronitor#ping_api_url which points to https://cronitor.link
• Cronitor#monitor_api_url which points to https://cronitor.io

These endpoints are currently hardcoded into the SDK.

This PR will extract them into the config file and allow them to be read from ENV variables:

• Cronitor#ping_url will read from ENV['CRONITOR_PING_URL'] and use https://cronitor.link as backup
• Cronitor#monitor_url will read from ENV['CRONITOR_MONITOR_URL'] and use https://cronitor.io as a backup.

The behaviour will not change for existing users. But it will allow for customising the endpoints by others (and me 😄)

*There is a third, Cronitor#fallback_ping_api_url which I have not touched since I can't imagine anyone wanting to change it

[aflanagan]

@willzoltan thanks for the PR! Can we make one small change and call them ping_url and monitor_url?

Could you also update this spec - https://github.com/cronitorio/cronitor-ruby/blob/master/spec/cronitor_spec.rb#L24 - to test for them being set/falling back to default?

[willzoltan]

@aflanagan Thanks! I've changed accordingly. Could you please run the workflow (only those with write access can)? Ping me if there are still issues. Also, I wrote you a note on drift about the endpoint you suggested not being valid 🙏

[willzoltan]

@aflanagan Specs should be fixed!

[aflanagan]

@willzoltan thank you for these! I think the specs need to be updated to not reference default_XXX since those aren't defined on the module.

[willzoltan]

sorry I forgot to commit all my changes... Fixed!

[willzoltan]

fixed again. Sorry I don't have the test suite set up on my computer

[aflanagan]

This is kinda nit-picky on my part, so take it with a grain of salt, but since it's reading from the ENV var, these aren't really "defaults". Could we keep setting these the same as before, and then in the test set up where these methods are used just set the defaults correctly there?

Or, if you want to keep these methods, just have them return the string only instead of calling ENV.fetch

[willzoltan]

I wasn't originally going to add these methods, but basically we need a method to reset the ping_url and the monitor_url from the test suite. This is because Crontior is like a god object, so when the first test sets the values of ping_url and the monitor_url, the second test retains the previous values. There is no way to flush or reload the Cronitor class.

But we need to load a fresh Cronitor class twice for the test suite. Once for when there are no ENV vars. And then again when there are ENV vars.

So these default_* methods allow me to call self.ping_url = default_ping_url before each test, just like they are called when the Cronitor class loads, so that when we change the ENV vars for the second test, the tests then behave properly and actually read the ENV vars instead of retaining the values from the first test.

We could copy the code which sets the initial ping_url and monitor_url values into the test suite and have something like below in the spec instead.

In my view this is worse, because it is not DRY and if the code in lib/cronitor/config.rb changes, then we are not testing the Cronitor class, but testing the code that is in the before block.

Would changing the name of the methods from default_* to initial_* be better?

describe '#ping_url and #monitor_url' do
  let(:env_vars) { ENV }

  before do
    stub_const('ENV', env_vars)
    # We need to reset the ping_url and monitor_url since the previous spec sets them globally
    Cronitor.configure do |cronitor|
      cronitor.ping_url = ENV.fetch('CRONITOR_PING_URL', 'https://cronitor.link') # copied from lib/cronitor/config.rb
      cronitor.monitor_url = ENV.fetch('CRONITOR_MONITOR_URL', 'https://cronitor.io') # copied from lib/cronitor/config.rb
    end
  end

  context 'when custom CRONITOR_PING_URL and CRONITOR_MONITOR_URL ENV variables are set' do
    let(:env_vars) do
      ENV.to_hash.merge(
        'CRONITOR_PING_URL' => 'https://ping.com',
        'CRONITOR_MONITOR_URL' => 'https://monitor.com'
      )
    end

    it 'returns the custom Cronitor URLs' do
      expect(Cronitor.ping_url).to eq('https://ping.com')
      expect(Cronitor.monitor_url).to eq('https://monitor.com')
    end
  end

  context 'when no CRONITOR_PING_URL or CRONITOR_MONITOR_URL ENV variables are set' do
    it 'returns the default Cronitor URLs' do
      expect(Cronitor.ping_url).to eq('https://cronitor.link')
      expect(Cronitor.monitor_url).to eq('https://cronitor.io')
    end
  end
end

[willzoltan]

@aflanagan

[aflanagan]

@willzoltan closing in favor or #28 which builds on your work in this PR.