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

Custom faraday and verify method #55

Merged
merged 9 commits into from
Aug 9, 2013
Merged

Custom faraday and verify method #55

merged 9 commits into from
Aug 9, 2013

Conversation

petems
Copy link
Owner

@petems petems commented Aug 6, 2013

I've somehow managed to refresh this page twice and lose the long description of what this is doing, so going for a tl;dr now...

Benefits

  1. Allows us more freedom with faraday, without having to make changes upstream on faraday itself or on digital_ocean
  2. Fixes Handle 401s more gracefully... #53, now uses the Faraday ClientError wrapper so you can return a clean response instead of a nil class stack trace (as seen in "undefined method `each' for nil:NilClass" in every endpoint #52)
  3. Adds a new verify option, just because it was a nice way of adding a test for my changes and its useful to check your credentials haven't been revoked without running authorize again.

@blom @pearkes any thoughts?

@pearkes
Copy link
Collaborator

pearkes commented Aug 7, 2013

Woo! This looks super awesome!

Overall, I'm good with this being merged. Addition of the verify command, real helpful.

One question – can I ask what will happen if we get say, a 503 error from DigitalOcean?

@petems
Copy link
Owner Author

petems commented Aug 7, 2013

Ah, that's the beauty of Faraday's raise error method, it slurps up all errors and will put out whatever error is given (Check it out here: https://github.com/lostisland/faraday/blob/master/lib/faraday/response/raise_error.rb)

So it will actually pick up edge cases like not being connected to the internet when using tugboat and the like:

Before:

tugboat sizes
/opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:763:in `initialize': getaddrinfo: nodename nor servname provided, or not known (Faraday::Error::ConnectionFailed)
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:763:in `open'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:763:in `block in connect'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/timeout.rb:55:in `timeout'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/timeout.rb:100:in `timeout'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:763:in `connect'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:756:in `do_start'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:745:in `start'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:1285:in `request'
    from /opt/boxen/rbenv/versions/1.9.3-p448/lib/ruby/1.9.1/net/http.rb:1027:in `get'

After:

tugboat sizes
Authentication with DigitalOcean failed (getaddrinfo: nodename nor servname provided, or not known)
 Check your API keys and run `tugboat authorize` to re-enter them if needed

If needed, we could make a case argument around the rescue that gives a different message for each error, but I thought I'd KISS for now 😄

@pearkes
Copy link
Collaborator

pearkes commented Aug 7, 2013

Sweet, that answers my question. 👍 Agree to keep it simple for now.

Pending any thoughts from @blom, this looks good to me!

@petems
Copy link
Owner Author

petems commented Aug 9, 2013

Cool, I'm gonna merge this then, any issues gimmie a shout 👍

petems added a commit that referenced this pull request Aug 9, 2013
Custom faraday parameters and verify method added
@petems petems merged commit 80be81b into master Aug 9, 2013
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.

Handle 401s more gracefully...
2 participants