Replace rest_client with faraday

[andrzej-stencel]

No description provided.

[grosser]

if we are going down this route then I'd recommend using faraday which comes with swappable backends

[cben]

Interesting, I was expecting we'll just switch over to a new library. At least in a major release, that sounds acceptable to me.

But if you're going to the trouble of supporting 2 libraries, then yes, Faraday makes sense.

[cben]

A strategic question I have about supporting multiple libraries, whether directly or via Faraday, is testing.

With time, we've seen there are corner cases where interaction with a specific library (e.g. kubeclient now doesn't catch exceptions that RestClient raises when a watch connection gets severed)...
If we have conditional code to support multiple backends, will anyone test them? Will it decay into each one having distinct bugs?

• Webmock does make a valiant effort to allow same tests to run on multiple libraries.

[grosser]

- behavior around watching might be different depending on backend library (or faraday backend), so that will need some debugging - whatever backend is supported needs to have all it's errors captured ... I'd assume that is not that hard

…

On Sun, Nov 1, 2020 at 2:45 PM Beni Cherniavsky-Paskin < ***@***.***> wrote: A strategic question I have about supporting multiple libraries, whether directly or via Faraday, is testing. With time, we've seen there are corner cases where interaction with a specific library (e.g. kubeclient now doesn't catch exceptions that RestClient raises when a watch connection gets severed)... If we have conditional code to support multiple backends, will anyone test them? Will it decay into each one having distinct bugs? - Webmock does make a valiant effort to allow same tests to run on multiple libraries. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ2UYIOPLG76MKDQQSLSNXQJDANCNFSM4TFCCN7Q> .

[andrzej-stencel]

Thanks a lot folks for looking at the code. I should probably clarify my approach for this PR.

I wanted to make this change as non-intrusive as possible, to prevent breaking changes (and thus be able to release sooner, maybe as 4.10?). That's why I decided to leave in rest_client by default. Since the instance of rest_client is available via a public accessor, people reading the docs (e.g. at RubyDocs) might assume it's OK to use it, and removing it or changing the underlying type could be considered a breaking change.

I was also contemplating adding the HTTPClient support as a separate gem, so that Kubeclient gem continues to use and support only rest_client, but via a wrapper that is easy to replace with another implementation by a separate gem. This would make it obvious where the tests need to be - Kubeclient could only care about testing with rest_client; the tests for a different library (and the responsibility to work as intended) would be on the separate gem. I was thinking something like:

require 'kubeclient'
require 'kubeclient-httpclient'

# The below instance would 'magically' use `httpclient` under the hood?
client = Kubeclient::Client.new('https://apiserver/api', 'v1')

I agree Faraday sounds like a good fit for this scenario because it already supports swappable backends. I'm a bit worried though this would be a breaking change, requiring a 5.0 release and this might take a while. Also I tend to think it's always best to avoid breaking changes at all, if possible. On the other hand, introducing complexity to avoid breaking changes doesn't sound too clever either 🤔

Please let me know what you think. If you'd rather have RestClient replaced with Faraday and releasing as 5.0, I'll go that path.

[grosser]

I guess another alternative would be to use net/http directly and avoid dependencies altogether. Adding complexity of multiple backends seems like a step in the wrong direction to me though. But either way (2 backends / faraday / net-http) getting feedback from the maintainers on what is mergeable is most important first :)

…

On Mon, Nov 2, 2020 at 1:10 AM Andrzej Stencel ***@***.***> wrote: Thanks a lot folks for looking at the code. I should probably clarify my approach for this PR. I wanted to make this change as non-intrusive as possible, to prevent breaking changes (and thus be able to release sooner, maybe as 4.10?). That's why I decided to leave in rest_client by default. Since the instance of rest_client is available via a public accessor, people reading the docs (e.g. at RubyDocs <https://www.rubydoc.info/gems/kubeclient/3.0.0/Kubeclient/ClientMixin#rest_client-instance_method>) might assume it's OK to use it, and removing it or changing the underlying type could be considered a breaking change. I was also contemplating adding the HTTPClient support as a separate gem, so that Kubeclient gem continues to use and support only rest_client, but via a wrapper that is easy to replace with another implementation by a separate gem. This would make it obvious where the tests need to be - Kubeclient could only care about testing with rest_client; the tests for a different library (and the responsibility to work as intended) would be on the separate gem. I was thinking something like: require 'kubeclient' require 'kubeclient-httpclient' # The below instance would 'magically' use `httpclient` under the hood? client = Kubeclient::Client.new('https://apiserver/api', 'v1') I agree Faraday sounds like a good fit for this scenario because it already supports swappable backends. I'm a bit worried though this would be a breaking change, requiring a 5.0 release and this might take a while. Also I tend to think it's always best to avoid breaking changes at all, if possible. On the other hand, introducing complexity to avoid breaking changes doesn't sound too clever either 🤔 Please let me know what you think. If you'd rather have RestClient replaced with Faraday and releasing as 5.0, I'll go that path. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ75ICB52MBHPUKRN7DSNZZO3ANCNFSM4TFCCN7Q> .

[cben]

I agree Faraday sounds like a good fit for this scenario because it already supports swappable backends. I'm a bit worried though this would be a breaking change, requiring a 5.0 release and this might take a while. Also I tend to think it's always best to avoid breaking changes at all, if possible. On the other hand, introducing complexity to avoid breaking changes doesn't sound too clever either

Hmm, I see your dilemma 🤔.
However I don't see 2 backends as a viable fast way to land this, as code-wise it feels like a dead end.

The reason I'm thinking of a major release is not that this should break anything — external API should remain same — but just that it might, and it's a big enough dependencies switch I think consumers of kubeclient should be conscious of it.
IMO a major version bump actually ticks off the same concerns that led you to implementing opt-in: people sort-of opt-in by upgrading, and may stay on rest-client for a while by holding kubeclient back.

Some thoughts, I'll write more later:

• rest-client is unmaintained for a long while (last commit 15 month ago, no community fork arose AFAIK).
  I think it's clear the end goal is to drop all rest-client code.

• http gem is still maintained, but one of the goals users wanted in switch http client to something that does not have a native extension #237 was pure ruby.

• I feel more comfortable backporting 5.x PRs to 4.x if people need them, than maintaining 2-backend conditionals.

• Personally I'm OK with Faraday and I'm OK with switching to a single successor library directly.
  From above discussion, I'm not sure whether any of us sees a real need for Faraday / multiple backends, it just came into the discussion because you started to implement a mini HTTP abstraction layer 😉

P.S. I'll mention I trust @grosser's opinion no less than my own 😀

[cben]

@astencel-sumo as you are doing the work, I think choice between Faraday or specific lib like httpclient is largely yours 💪
Either way would be a clear improvement on current state.

• You told me in an email you're working on this primarily to achieve HTTPS connection reuse (which would be great to have!).
  Let us know if any of the options are unacceptable due to this.

• I believe there are use cases where people want control over connection reuse, though I don't remember concrete examples 🤷
  I recommend for first PR not activating connection reuse by default. Make it opt-in in some way, or even don't expose it at all yet; will be easier to discuss separately.

• A weird but pragmatic way choose between Faraday and directly using a single lib:
  Are you certain which lib you want and that it'll satisfy your and our needs? If not, Faraday lets you postpone the decision 😆

You maybe saw "5.0" plans in #435. Don't worry about these much — these were things I wanted to implement by 5.0, and didn't make enough progress; if you get this working first, that alone justifies a quick 5.0 release 👍

• I have no qualms breaking people that used .rest_client in 5.0. Either you'll drop it or I'll make it private, but consider it gone ✂️.

[cben]

Hmm, httpclient last release was in 2016, last commit in Feb 2019.

Does that worry us 😟 or has it reached perfection ✨ ?
nahi/httpclient#418

[grosser]

FYI for persistent net http https://github.com/drbrain/net-http-persistent

... but that would also be a gem requirement, so might as well pick faraday and whoever needs to can use it with persistent backend, I'm mostly worried that watch might not work or have weird exceptions, would definetely be easier to maintain a single backend like restclient (or http + persistent)

[andrzej-stencel]

Just a note off the top of my head - a feature I imagine one might find valueable is support for HTTP/2, which Kubernetes API server supports but not many Ruby HTTP libraries do - correct me if I'm wrong, but net/http currently cannot do HTTP/2, can it?

[andrzej-stencel]

Hey folks, I've been doing some more checking and I it seems two options have the most advantages:

1. Least dependencies: just use 'net/http', I don't even think 'net-http-persistent' is needed for basic keep-alive to one host (the Net::HTTP#start method should be enough). The disadvantage is 'net/http' API is clunky and a bit hard to write clean code with. Also is a bit bare-bones - need to implement request forwarding on 3xx, raising errors on 4xx/5xx.

2. Use Faraday with 'net-http-persistent' adapter - more features (has middleware for errors and request forwarding), but means two additional dependencies, more complexity when dealing with issues (is it a problem in kubeclient? faraday? net-http-persistent? 'net/http' which is still under the hood?).

I have a hard time deciding between these two options. I would love to not have to deal with the 'net/http' API and not have to re-invent features like request forwarding and just use a tool that does it all. On the other hand, not having two additional dependencies has value as well.

Also, as not an active Ruby scene member, I cannot judge how good a choice Faraday is. Seems to be pretty popular as a matter of fact.

I would like to ask for your opinions @cben , @grosser - and anybody else who might have something to say.

[grosser]

for net-http: - not sure if we even need forwarding - can steal most code from faradays net/http adapter - I'd like to see what a PR with it looks like (without tests for speed), but might be not that bad - using .start might lead to interesting bugs like the connection going bad (what net-http-persistent deals with) for faraday: - connection reuse does not happen here natively, so need the persistent gem to make that work, but that can be on the end-user to require - not sure how/if watch would work, but I'd assume it's not that hard - I'd assume the implementation would be pretty simple it could be interesting to extract an internal http api/object first to make it obvious which features we need and also hide all the complexity and then make PRs on top of that

…

On Wed, Nov 18, 2020 at 5:33 AM Andrzej Stencel ***@***.***> wrote: Hey folks, I've been doing some more checking and I it seems two options have the most advantages: 1. Least dependencies: just use 'net/http', I don't even think 'net-http-persistent' is needed for basic keep-alive to one host (the Net::HTTP#start method should be enough). The disadvantage is 'net/http' API is clunky and a bit hard to write clean code with. Also is a bit bare-bones - need to implement request forwarding on 3xx, raising errors on 4xx/5xx. 2. Use Faraday with 'net-http-persistent' adapter - more features (has middleware for errors and request forwarding), but means two additional dependencies, more complexity when dealing with issues (is it a problem in kubeclient? faraday? net-http-persistent? 'net/http' which is still under the hood?). I have a hard time deciding between these two options. I would love to not have to deal with the 'net/http' API and not have to re-invent features like request forwarding and just use a tool that does it all. On the other hand, not having two additional dependencies has value as well. Also, as not an active Ruby scene member, I cannot judge how good a choice Faraday is. Seems to be pretty popular as a matter of fact. I would like to ask for your opinions @cben <https://github.com/cben> , @grosser <https://github.com/grosser> - and anybody else who might have something to say. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ5PGJVZSAC4HV44QZLSQPEJNANCNFSM4TFCCN7Q> .

[andrzej-stencel]

extract an internal http api/object first to

That's the HTTPWrapper class from my PR :) Not necessarily as an abstract class/interface though, but rather as a single concrete implementation.

[andrzej-stencel]

This PR in current state is using faraday everywhere where rest_client has been used. I haven't replaced the other gem, http. Faraday is being used with the default plain net/http adapter, so no persistent connections here either.

❓ 1️⃣ Do we want to support the net_http_persistent adapter (and persistent connections) as an out-of-the-box option or do we just want to make it possible for end user to replace the Faraday backed at their own risk?

❓ 2️⃣ Should I be replacing the http gem in this PR as well?

[grosser]

this should not be necessary and can mess with require hacks, so prefer to avoid

[andrzej-stencel]

Not sure what you refer to with this? Is it all the require => require_relative changes? It's just been merged on master, so needs to reverted if it's not the best idea after all.

@grosser can you point me to where I can read more about this?

[grosser]

I don't have a good article for it, just know I ran into lots of trouble when combining this with bootsnap and other things the override required ... if it solves a real issue then I guess leave it until we have more concrete evidence rebase would also be good to clean this diff up then :)

…

On Sun, Nov 22, 2020 at 1:48 PM Andrzej Stencel ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/kubeclient.rb <#466 (comment)>: > -require 'kubeclient/aws_eks_credentials' -require 'kubeclient/common' -require 'kubeclient/config' -require 'kubeclient/entity_list' -require 'kubeclient/exec_credentials' -require 'kubeclient/gcp_auth_provider' -require 'kubeclient/http_error' -require 'kubeclient/missing_kind_compatibility' -require 'kubeclient/oidc_auth_provider' -require 'kubeclient/resource' -require 'kubeclient/resource_not_found_error' -require 'kubeclient/version' -require 'kubeclient/watch_stream' +require_relative 'kubeclient/aws_eks_credentials' Not sure what you refer to with this? Is it all the require => require_relative changes? It's just been merged on master, so needs to reverted if it's not the best idea after all. @grosser <https://github.com/grosser> can you point me to where I can read more about this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZZ35JKDNQQSVDIOCJLSRGBK7ANCNFSM4TFCCN7Q> .

[grosser]

FYI tried this on one of our projects that use kubeclient heavily with gem 'kubeclient', git: 'https://github.com/astencel-sumo/kubeclient.git', branch: 'change-underlying-http-library' no errors or change in cpu/memory usage after 2 hours 👍

[grosser]

Found timeouts failing their to_s

Net::OpenTimeout: Net::OpenTimeout
  File "/usr/local/lib/ruby/2.6.0/net/protocol.rb", line 41, in ssl_socket_connect
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 996, in connect
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 930, in do_start
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 919, in start
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 144, in request_via_get_method
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 135, in request_with_wrapped_block
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 128, in perform_request
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 70, in block in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter.rb", line 61, in connection
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 68, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/response.rb", line 11, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday_middleware-1.0.0/lib/faraday_middleware/response/follow_redirects.rb", line 79, in perform_with_redirection
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday_middleware-1.0.0/lib/faraday_middleware/response/follow_redirects.rb", line 67, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/rack_builder.rb", line 154, in build_response
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/connection.rb", line 492, in run_request
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/connection.rb", line 198, in get
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 373, in block in get_entities
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 125, in handle_exception
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 372, in get_entities
...
Faraday::ConnectionFailed: Net::OpenTimeout
  File "/usr/local/lib/ruby/2.6.0/net/protocol.rb", line 41, in ssl_socket_connect
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 996, in connect
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 930, in do_start
  File "/usr/local/lib/ruby/2.6.0/net/http.rb", line 919, in start
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 144, in request_via_get_method
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 135, in request_with_wrapped_block
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 128, in perform_request
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 70, in block in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter.rb", line 61, in connection
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/adapter/net_http.rb", line 68, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/response.rb", line 11, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday_middleware-1.0.0/lib/faraday_middleware/response/follow_redirects.rb", line 79, in perform_with_redirection
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday_middleware-1.0.0/lib/faraday_middleware/response/follow_redirects.rb", line 67, in call
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/rack_builder.rb", line 154, in build_response
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/connection.rb", line 492, in run_request
  File "/app/vendor/bundle/ruby/2.6.0/gems/faraday-1.1.0/lib/faraday/connection.rb", line 198, in get
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 373, in block in get_entities
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 125, in handle_exception
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 372, in get_entities
...
Kubeclient::HttpError: Net::OpenTimeout
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 130, in rescue in handle_exception
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 124, in handle_exception
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/common.rb", line 372, in get_entities
...
NoMethodError: undefined method `[]' for nil:NilClass
  File "/app/vendor/bundle/ruby/2.6.0/bundler/gems/kubeclient-cd78265bcc51/lib/kubeclient/http_error.rb", line 18, in to_s
...

[andrzej-stencel]

Thanks @grosser, this is now hopefully fixed with commit 7de1209.

As a side note, error handling certainly needs some love. Would be good to pass the original exception to the user (even if wrapped in a Kubeclient exception), not only the HTTP status, message and response (of which both HTTP status and response might be empty).

[andrzej-stencel]

@cben can you have a look at this PR and the comment above?

@grosser is there something you'd like to see changed in this PR to be approved?

[cben]

Sorry for radio silence again 😳
This is great! 👏

❓ 2️⃣ Should I be replacing the http gem in this PR as well?

PR split is up to you. Can add commits here, can keep this open and open a 2nd PR building on top of this, or can merge this first.

[cben]

Please mark this, and everything faraday-specific, private. (feel free to re-order them if you want)

As already discussed, breaking people that used undocumented .create_rest_client / .rest_client is fine 👍.
Perhaps in future we'll expose some or parts of it — this is tempting with Faraday. Maybe we'll keep it abstracted (#389). Maybe we'll instead convenience methods on Config (#417). For now making it private keeps our options open...

naming: currently, variables and methods called http_... mostly referred to the http gem (as opposed to rest-client). But we'll get rid of http and anyway that was confusing usage, I'm happy to reclaim it to just mean the HTTP protocol 👍

[andrzej-stencel]

Having .http_client public makes it super-easy to replace the Faraday adapter with net/http/persistent by the users of kubeclient. I can now do this like this:

client = Kubeclient::Client.new(...)
client.http_client.adapter(:net_http_persistent)

This way I get HTTP connection persistence without having it in upstream Kubeclient.

Yes I would really like to be able to get HTTP connection persistence with Kubeclient 5.0, that's the whole point, or the business driver if you will :)

[cben]

❓ 1️⃣ Do we want to support the net_http_persistent adapter (and persistent connections) as an out-of-the-box option or do we just want to make it possible for end user to replace the Faraday backed at their own risk?

I'm not sure. Let's merge basic Faraday conversion first.
If it helps, we could even release 5.0 with just Faraday and add persistent support in 5.1! But I suspect you'll want persistent support proven first, as that was your goal.

In any case, I'd like persistence to be optional. Not sure if opt-in or opt-out but if people have issues they should have a way to rule it out.

How many configuration knobs persistence needs?
I see https://lostisland.github.io/faraday/adapters/net-http-persistent offers pool_size and idle_timeout.

https://github.com/lostisland/faraday/blob/master/lib/faraday/adapter/net_http_persistent.rb uses a Net::HTTP::Persistent instance per faraday connection.
Should Kubeclient users have control over separate/shared connections? It seems eventually all libraries for persistence tend to expose such control.
Do we use 1 connection per Client? That way though there will be no reuse between separate Client objects for separate API groups :-(

[cben]

Having .http_client public makes it super-easy to replace the Faraday adapter with net/http/persistent by the users of kubeclient. I can now do this like this:

client = Kubeclient::Client.new(...)
client.http_client.adapter(:net_http_persistent)

Oh, I see. OK, we can do that.
Consider calling the method .faraday_client or .faraday_connection to be clearer what's the interface this exposes.

What about multiple Kubeclient::Client objects (same server, different API groups)? Will they magically reuse persistent connections too?

What about watch? Watch requests are long-lived, so it ties up one HTTP connection, and there is no meaningful persistence between watches, right? (At least until k8s gets HTTP/2)
Suppose you have a pool size of 10 and open 15 watches, will they "starve" the pool and cause regular requests to not reuse connections? But DEFAULT_POOL_SIZE is pretty large so I don't think that really matters 🤷‍♂️

[cben]

Anyway, LGTM, let's progress from here 👍

[andrzej-stencel]

Oh great, thanks for merging @cben! Good points about watching and connection per api group, I'll continue this on Monday if that's OK.

[andrzej-stencel]

Hey @cben,

Let me reply in points.

Consider calling the method .faraday_client or .faraday_connection to be clearer what's the interface this exposes.

Sure, will do.

What about multiple Kubeclient::Client objects (same server, different API groups)? Will they magically reuse persistent connections too?

I'm afraid not.

What about watch? Watch requests are long-lived, so it ties up one HTTP connection, and there is no meaningful persistence between watches, right? (At least until k8s gets HTTP/2)

This exact PR does not touch watch requests, as those are served by the http gem. We could probably replace http with faraday as well, to use single tool for all HTTP requests. Let's discuss this in a separate thread (issue or PR), if that's OK?