broadway_cloud_pub_sub icon indicating copy to clipboard operation
broadway_cloud_pub_sub copied to clipboard

Consider supporting PUBSUB_EMULATOR_HOST

Open mcrumm opened this issue 5 years ago • 10 comments

There's a lot of prior art for overriding authentication in the presence of an PUBSUB_EMULATOR_HOST environment variable. I'm opening this issue to get feedback on supporting it for the default GoogleApiClient implementation.

Basically we'd look for the var at runtime, and when present, use its value as the base url and set a fake token generator.

This will need to be per-implementation, but we have prepare_to_connect on the Client behaviour for that.

The only thing I'm not wild about is that the GoogleApiClient implementation would need to reach into the :google_api_pub_sub app config to override the base_url.

Still, personally I'd like to see this implemented, as the required configuration steps are non-trivial, especially if you're coming from somewhere like the Python PubSub client, where you can just set an env var to use the emulator.

Original comment: https://github.com/peburrows/goth/issues/56#issuecomment-577864877

/cc @matehat

mcrumm avatar Jan 25 '20 04:01 mcrumm

Is this something we could push to be supported upstream?

josevalim avatar Jan 25 '20 08:01 josevalim

It's worth a shot! Upstream I'd like to see more runtime config for the Tesla client. That's probably 90% of the difficulty right now.

mcrumm avatar Jan 25 '20 16:01 mcrumm

I opened up an issue a long time ago on Tesla exactly about the compile time config issues in Google APIs. Those issues have been addressed, so it may be a matter of updating Tesla and moving away from the compile time API.

josevalim avatar Jan 25 '20 16:01 josevalim

Ah, yes - and the Google API issue is still open: https://github.com/googleapis/elixir-google-api/issues/98 :-)

The Tesla version in latest is relatively up-to-date (v1.2); the problem right now is clients use GoogleApi.Gax.Connection, which only generates new/0 and new/1.

A function on the Connection (maybe new/2?) that exposes more Tesla.client/2 options would allow us to override the HTTP adapter in the proper way, and possibly inject additional middleware (like Telemetry).

For emulator support directly in the client, perhaps the new/* functions need to be overridable when you use Connection so that clients in general, but PubSub specifically, could override the base_url at runtime using the env var. Thoughts?

(Edited to clarify needing to override base_url at runtime)

mcrumm avatar Jan 25 '20 17:01 mcrumm

Yeah, something along those lines. Maybe we should not make all new/* but rather have then build on top of a single build_client overridable one.

FWIW, we can always force them to be overridable By calling defoverridable ourselves, but having both scenarios supported upstream would be nicer. :)

josevalim avatar Jan 25 '20 18:01 josevalim

This will at least allow you to use the emulator by configuring it in dev.exs, you could use this to pull from the env var.

config :google_api_pub_sub, base_url: System.get_env("PUBSUB_EMULATOR_HOST", "http://localhost:8085")

michaelst avatar Dec 02 '20 20:12 michaelst

Is there any progress on this? I've tried @michaelst's suggestion but it's not working for me - I can't see anywhere in the code that the base_url is read from Config

christopherdbull avatar Apr 06 '21 16:04 christopherdbull

I think that config workaround should work:

The google_api_pub_sub connection setups the default base_url here: https://github.com/googleapis/elixir-google-api/blob/master/clients/pub_sub/lib/google_api/pub_sub/v1/connection.ex#L34

And Gax.Connection allow to override it with the configuration: https://github.com/googleapis/elixir-google-api/blob/master/clients/gax/lib/google_api/gax/connection.ex#L27

rockneurotiko avatar Apr 06 '21 17:04 rockneurotiko

Ok, more digging and looks like this is it: https://github.com/peburrows/goth/issues/56 Goth still tried to get the token, but the config actually worked. Thanks for your help, but would be good if broadway could support this automatically.

christopherdbull avatar Apr 07 '21 08:04 christopherdbull

I'd love to see PUBSUB_EMULATOR_HOST supported out of the box. Right now it's my second hour digging into connecting Broadway to the emulator and I'm still somewhere half-way there.

lessless avatar Oct 04 '22 15:10 lessless