martian-httpkit defaults to async

[onetom]

Other HTTP library adapters default to synchronous operation, but since httpkit is async by default. This inconsistency prohibits easy switching between HTTP libs, like this:

    [martian.httpkit :as martian-http]
    #_[martian.clj-http :as martian-http]

For the sake of consistency, I would think the httpkit implementation should default to synchronous mode too.

This issue came up for me, because we are using Martian in a Datomic Cloud environment, where the versions of a bunch of libraries are controlled. To minimize version conflict, we are trying to keep the our own app's dependency count low. One way to achieve this is to use httpkit instead of clj-http and once we upgrade to the latest Datomic Cloud version, we can switch to hato.

[onetom]

This would be a breaking change though, so alternatively, this inconsistency can be just highlighted in the documentation. If there would be a sync version of the martian.httpkit/perform-request interceptor, like in martian.hato, then the documentation can just contain a snippet about how to use that.

Unfortunately, in martian.hato the name of the sync and async interceptors differ. This make the use of martian.interceptor/inject dependant on whether we use perform-request or perform-request-async.

To get around this problem, we can tweak final-result in martian.interceptors/inject to perform an isa? operation, instead of =, allowing (inject ,,, martian.httpkit/preform-request-sync :replace :perform/request), given something like this:

(derive :perform.sync/request :perform/request)
(derive :perform.async/request :perform/request)

(isa? :perform.sync/request :perform/request) ;=> true
(isa? :perform.async/request :perform/request) ;=> true
(isa? :perform/request :perform/request) ;=> true

Furthermore, using sieppari instead of tripod might also reduce the code required for handling the differences between sync and async perform-request implementations. (I don't have much experience with these libraries - or interceptors in general - though, so it's just a gut feeling.)

[onetom]

I guess the missing interceptor would be just:

(def perform-request-sync
  {:name ::perform-request-sync
   :leave (fn [{:keys [request] :as ctx}]
            (-> ctx (assoc :response @(http/request request))))})

[oliyh]

Hi,

Yes, I hadn't actually considered that martian could just do the blocking for you and make the API more consistent. My thinking was that it didn't hide the http library's implementation details from you, because you might have chosen http-kit because you wanted the async behaviour (at a time when clj-http did not have async available, and hato didn't exist).

It would be a breaking change to default to that so I think I might avoid that, but it would certainly be possible to make a sync perform-request interceptor. As you mention, perhaps using keyword inheritance would allow them both to work better with the inject function.

[fhalde]

hello, curious, by any chance do we have to call tc/execute in the sync case?

(def perform-request-sync
  {:name ::perform-request-sync
   :leave (fn [{:keys [request] :as ctx}]
            (-> ctx (assoc :response @(http/request request)) tc/execute))})

the async interceptor calls this. we don't know what's the purpose of tc/execute and the go-async @onetom @oliyh

[oliyh]

Hi @fhalde

The purpose of the go-async and tc/execute in the async case is to run the 'leave' phase of the interceptor stack (deserialising, validation etc). It's exposed there because it's offloading into another thread to be run in the future when the response comes back.

When we are waiting on the response on the same thread in the sync case, you should not call execute, as that will be done for you.