Strech / avrora

A convenient Elixir library to work with Avro messages, schemas and Confluentยฎ Schema Registry
https://hexdocs.pm/avrora
MIT License
98 stars 33 forks source link

Private clients hardcode schemas path at compile time #88

Closed LostKobrakai closed 3 years ago

LostKobrakai commented 3 years ago

Following up on the discussion at https://github.com/Strech/avrora/pull/86:

I've made a repo producing the issue: https://github.com/LostKobrakai/avrora_repro

I wasn't sure if us using avrora as a dependency was the reason, so I started with an umbrella, but that should actually be irrelevant. The important part is removing _build as the path, which was hardcoded at compile time. The steps are documented in the readme of the repo.

Strech commented 3 years ago

๐Ÿ‘‹๐Ÿผ @LostKobrakai Thanks a lot for your effort, I will take a look and let's discuss what we can do about it in this issue ๐Ÿ‘๐Ÿผ

Strech commented 3 years ago

Ok, I can confirm that I see the issue with umbrella releases and 2 different clients root dirs. Let me read a bit about umbrella releases

Strech commented 3 years ago

After a few modifications of the issue reproduction repo, I would like to propose the same way as Ecto is doing it.

defmodule ReproB.Client do
  use Avrora.Client,
    otp_app: :repro_b,
    schemas_path: "./priv/schemas"
end

An :otp_app option will be for the private client only, since a shared client can't be used with more than 1 app. I've tested and it seems working fine.

Does it make sense to you? WDYT?

LostKobrakai commented 3 years ago

The umbrella part here should really be irrelevant. The important bit is that Application.app_dir at compile time will point at _build/โ€ฆ, which will neither be available to releases once moved to another machine, nor when the application is used as a dependency in another project. Plain relative paths do not work given mix and releases use different folder structures.

The solution will involve running Application.app_dir at runtime instead of compile time in some form. How we supply the inputs for that doesn't really matter to me.

Strech commented 3 years ago

Yeah, I get it. It actually still relevant for both Avrora and its private clients. I will push proposed configuration this week ๐Ÿ—“๏ธ

Strech commented 3 years ago

Heads up @LostKobrakai, I would like to say thanks for your great support, it was very helpful to have an example app. A new configuration option was added to the private client and shared client โ€“ otp_app, which will be used to resolve the path to the schemas in a runtime. Thanks ๐ŸŽ‰ ๐Ÿพ