Remove bare Net HTTP and make compatible with Faraday clients.

[pschambacher]

🐙💻

/cc @grosser @anamartinez @jcheatham @gabetax

Also interested in @ciaranarcher 's opinion since we're aiming to use our very own custom Faraday connection for this.

Description

Remove Net::HTTP implementation in favor of Faraday clients.

This should allow us to configure the gem with a Faraday connection and use it, benefiting from all the middlewares coming with it.

I'm just a bit doubtful about the SSL-bit since Faraday doesn't exactly allow us to change the configuration on the fly but I think it should work.

Steps to merge

• 👍 from the team

Risks

• High: this will be a major bump, the entire gem might not be working at all (I'm not entirely sure how much we can trust the test suite)

[grosser]

👍
FYI wanted to recommend hurley ... but apparently that is dead now too :D
lostisland/hurley#43

[jcheatham]

Yeah, definitely major bump; as a sanity check I'd recommend running a branch of classic with this, though unfortunately a most of the tests there are stubbed anyway, so it probably won't help much... no obvious red flags stand out to me, will take another pass but looks 👍

[jish]

https://www.mikeperham.com/2016/02/09/kill-your-dependencies/

Josh

On May 4, 2016, at 16:50, Pierre Schambacher [email protected] wrote:

Description

Remove Net::HTTP implementation in favor of Faraday clients.

This should allow us to configure the gem with a Faraday connection and use it, benefiting from all the middlewares coming with it.

I'm just a bit doubtful about the SSL-bit since Faraday doesn't exactly allow us to change the configuration on the fly but I think it should work.

Steps to merge

from the team
Risks

High: this will be a major bump, the entire gem might not be working at all (I'm not entirely sure how much we can trust the test suite)
You can view, comment on, or merge this pull request online at:

#23

Commit Summary

Remove bare Net HTTP and make compatible with Faraday clients.
File Changes

A .rspec (1)
M Gemfile (2)
M Gemfile.lock (6)
A lib/ticket_sharing.rb (9)
M lib/ticket_sharing/agreement.rb (4)
M lib/ticket_sharing/client.rb (9)
M lib/ticket_sharing/request.rb (76)
M lib/ticket_sharing/ticket.rb (6)
M spec/models/client_spec.rb (16)
M spec/models/request_spec.rb (8)
M spec/spec_helper.rb (1)
M ticket_sharing.gemspec (3)
Patch Links:

https://github.com/zendesk/ticket_sharing/pull/23.patch
https://github.com/zendesk/ticket_sharing/pull/23.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

[ciaranarcher]

Thanks for pinging me on this!

If you are going this far, why not use the recently merged Kragle connection so you get metrics etc for free 😍

The service in question could just be called "ticket_sharing". You could set up a ticket sharing "client" wrapping a Kragle connection (which is just a faraday connection with some defaults and metrics) which will set default headers / timeouts etc.

[pschambacher]

@ciaranarcher I'd love to but there a problem of public vs private gem here
:) that's why I defined TicketSharing.connection

On Thursday, 5 May 2016, Ciarán Archer [email protected] wrote:

Thanks for pinging me on this!

If you are going this far, why not use the recently merged
https://github.com/zendesk/zendesk/pull/22200 Kragle connection
https://github.com/zendesk/zendesk/blob/4b0f63237823b5455de9ee71766bd2e605db1fd5/lib/kragle_connection.rb
so you get metrics etc for free 😍

The service in question could just be called "ticket_sharing". You could
set up a ticket sharing "client" wrapping a Kragle connection (which is
just a faraday connection with some defaults and metrics) which will set
default headers / timeouts etc.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#23 (comment)