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

Supports explicit endpoint configuration #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jkassemi
Copy link

config block all the things

@parasquid
Copy link
Owner

I think it would be better if when config.endpoint is blank, it will auto detect the environment and auto-fill the url accordingly. In other words, use config.endpoint when available otherwise fallback to original behavior.

Why do you need to only allow explicit endpoint configuration? :)

@jkassemi
Copy link
Author

Howdy @parasquid!

The changes support explicit endpoint configuration, but they do fall back to implicit by default:

require 'helper'

describe Namecheap::Api do
  before { set_dummy_config }

  it 'uses sandbox for development environments' do
    stub_const('Rails', double(env: 'development'))
    load File.join(File.dirname(__FILE__), '../../lib/namecheap/api.rb')

    expect(HTTParty).to receive(:get).with(Namecheap::Api::SANDBOX, anything())
    subject.request('get', 'world', {})
  end

  it 'uses sandbox for unspecified environment' do
    load File.join(File.dirname(__FILE__), '../../lib/namecheap/api.rb')
    expect(HTTParty).to receive(:get).with(Namecheap::Api::SANDBOX, anything())
    subject.request('get', 'world', {})
  end

  it 'uses production for production environments' do
    stub_const('Rails', double(env: 'production'))
    load File.join(File.dirname(__FILE__), '../../lib/namecheap/api.rb')
    expect(HTTParty).to receive(:get).with(Namecheap::Api::PRODUCTION, anything())
    subject.request('get', 'world', {})
  end

  it 'directs the endpoint where specified' do
    Namecheap.config.endpoint = 'http://example.com'
    load File.join(File.dirname(__FILE__), '../../lib/namecheap/api.rb')
    expect(HTTParty).to receive(:get).with('http://example.com', anything())
    subject.request('get', 'world', {})
    Namecheap.config.endpoint = nil
  end
end

The above aren't included because I'm not a fan of the 'load' pattern I had to use.

I generally prefer the explicit definition on a feature by feature basis (Twelve-Factor App Config and dotenv), but in this case specifically I'm running Rails in production mode on a staging server and would still like that system to call out to Namecheap's sandbox ;)

It could make testing this easier if we'd refactor Namecheap::Api{#init_args, SANDBOX, PRODUCTION, ENVIRONMENT, ENDPOINT} into Namecheap::Config and avoid configuration state on load. What do you think?

@parasquid
Copy link
Owner

Sounds cool, I was just concerned that this pr will be a breaking change that will affect backwards compatibility.

I also agree with you, so how about we bump the major version number with this one? And probably temporarily maintain two versions and back port new features.

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.

2 participants