dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 411 forks source link

Example code in README for manual HttpClient build doesn't work #530

Closed wmatson closed 4 years ago

wmatson commented 4 years ago
;; You can also build your own, using clj-http's helper or manually building it:
(let [cm (conn/make-reusable-conn-manager {})
      hclient (core/build-http-client {} cm "https://example.com" false)]
  (client/get "http://example.com/1"
              {:connection-manager cm :http-client hclient})
  (client/get "http://example.com/2"
              {:connection-manager cm :http-client hclient})
  (client/get "http://example.com/3"
              {:connection-manager cm :http-client hclient}))

core/build-http-client takes a boolean for caching? before the connection-manager and the url supplied isn't necessary.

wmatson commented 4 years ago

I can probably make a PR to fix it, I just need to find the time. Things seem to work without specifying the connection-manager in addition to the http-client, is that intended or was I just lucky?

dakrone commented 4 years ago

@wmatson a PR for this would be great. I'm not sure what the second part of your question refers to, can you clarify?

wmatson commented 4 years ago
(let [cm (conn/make-reusable-conn-manager {})
      hclient (core/build-http-client {} false cm)]
  (client/get "http://example.com/1" {:http-client hclient}))

This "works", but the documentation implies that it shouldn't:

Note that in order to reuse the client a connection manager must be used.

wmatson commented 4 years ago

@dakrone I looked through the source and found that it's "harmless" to not supply the connection manager as specified in my previous comment's code snippet, although it does cause an unused connection manager to be created and shutdown.

Although it's impossible to get the connection manager from the HttpClient with the non-deprecated HttpClient API, it might be possible to not create one on request invocation if :http-client is supplied. Would you like me to add that to this PR or create a separate issue/PR for it?