search

dashbitco / broadway_cloud_pub_sub

A Broadway producer for Google Cloud Pub/Sub
Apache License 2.0
70 stars 24 forks source link

Support other HTTP adapters #51

Closed v0idpwn closed 2 years ago

v0idpwn commented 3 years ago

Hi! Before anything, thanks for the awesome work on this producer.

Currently, it seems like broadway_cloud_pub_sub only supports Hackney because it relies on :hackney_pool for pooling. I think it could support support other HTTP adapters, even if only one or two, as having hackney as a hard dependency may be a problem.

Today, https://github.com/benoitc/hackney/issues/643 is stopping my broadway_cloud_pub_sub from getting connections, and I'd like to be able to use another client.

I'd be happy to open a PR

josevalim commented 3 years ago

A PR is welcome. It seems to be a matter of making some options, such as __internal_tesla_adapter__, public.

v0idpwn commented 3 years ago

Nice! Will work on that once I finish some priorities.

sorliem commented 3 years ago

+1 would love to see Finch as an option, or the default

v0idpwn commented 3 years ago

I'm working on this (after 9 months!). Exposing __internal_tesla_adapter__ isn't enough because the client relies on hackney's pooling atm.

I was thinking of making a behaviour for HTTP adapters equal to the one implemented on https://github.com/peburrows/goth/pull/88, so broadway_cloud_pub_sub users could share the implementation between the two libraries, but I think this may introduce some undesired indirect coupling. WDYT?

mcrumm commented 3 years ago

@v0idpwn One option might be to introspect the adapter in GoogleApiClient.prepare_to_connect/2 and only return the pool spec if the adapter is Hackney.

Ideally we want to reduce the number of times a Message gets copied, so (in that regard) the less processes the better. I would be very curious to see how well the Mint adapter for Tesla would work in BroadwayCloudPubSub.

wojtekmach commented 3 years ago

Yeah given we have a pool of producers, we don't need a pooled HTTP client. Instead of, or in addition to, pursuing an HTTP contract, I think we could switch from Hackney to Mint.

https://dashbit.co/blog/building-a-custom-broadway-producer-for-the-twitter-stream-api can be helpful.