Strech / avrora

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

Load app config when `--module` arg is used #102

Closed emilianobovetti closed 1 year ago

emilianobovetti commented 1 year ago

Hello! I'm having troubles with mix avrora.reg.schema when I use the --module argument and my avrora client is configured with a runtime.exs.

I'll try to illustrate the problem, I have an avrora client like this one:

defmodule MyApp.Avrora do
  use Avrora.Client, otp_app: :my_app
end

and I'm using a runtime.exs

import Config

config :my_app, MyApp.Avrora,
  registry_url: System.fetch_env!("SCHEMA_REGISTRY_URL"),
  registry_auth:
    {:basic,
     [
       System.fetch_env!("SCHEMA_REGISTRY_USER"),
       System.fetch_env!("SCHEMA_REGISTRY_PASS")
     ]}

now the problem is that my_app is not being loaded by mix avrora.reg.schema --all --module MyApp.Avrora, so I get the following:

schema `my.namespace.SomeEvent' will be skipped due to an error `unconfigured_registry_url'

due to Application.get_env/3 returning nil when called with :registry_url key.

It looks like this could be solved by running Mix.Task.run("app.config") when --module command line argument is used (which I believe is not helpful otherwise).

cheers : )

Strech commented 1 year ago

Hey @emilianobovetti 👋🏼 thanks for your contribution!

I would like to ask about your config. Avrora is reading from the App config, which is dynamic, and you are using static values which will compile once and then used to configure App config.

Could you please double check that once you clean everything and compile with environment variables set it's still nil value?

emilianobovetti commented 1 year ago

hello and happy new year :)

you are using static values which will compile once and then used to configure App config

the problem here is that I'm using a config/runtime.exs to load those values, in fact I can confirm that using config/config.exs works as expected, but values in runtime.exs are loaded right before application starts.

I also found this thread on elixir forum where they describe this exact problem and came up with the same solution: https://elixirforum.com/t/config-runtime-exs-not-loaded-on-mix-tasks/38034

I can provide a repo to reproduce this issue if that's helpful.

Strech commented 1 year ago

If it doesn't bother you a small repo to play around will be awesome. I would like to dig a bit into that and see what's the reason behind it.

Maybe it's something that we are missing about the order of loading, before we give a try of your fix, will be cool to experiment a bit

emilianobovetti commented 1 year ago

I've put together this thing https://github.com/emilianobovetti/avrora_runtime_config

the configuration is here

the gist is mix avrora.reg.schema --all --module AvroraRuntimeConfig.AvroraClient gives an error

let me know if works for you

Strech commented 1 year ago

Hey @emilianobovetti, sorry for the long silence. Thanks a lot for providing an example repo. After some digging, I learned that technically your suggested changes are what we need to do, but the way it's done is not backward compatible and will only work starting Elixir 1.11.

The issue is that runtime.exs will never be loaded because it's not a default (or known) configuration to Elixir. And in order to achieve that we must either import it in config.exs or load it separately by ourselves.

And with the latter what we try to do we have few issues. First of all, it might be a different config for someone else and second – all the helpers come starting Elixir 1.11 and we should stay backward compatible with something older like Elixir 1.9.

My suggestion is to add --add-config or something like that as an option and load it if provided instead of always loading (because we have a default that is always loaded). So I/m gonna push some changes to your PR if you don't mind

emilianobovetti commented 1 year ago

Hei! Thanks for your exploratory work and for giving all this context. Of course feel free to push changes or close this pr if you prefer I don't mind at all!

Strech commented 1 year ago

With newly pushed changes registration did work as designed 🎉

22:15:23.849 [debug] reading schema `playground.HelloWorld` from the file /Users/.../_build/dev/lib/avrora_runtime_config/priv/schemas/playground/HelloWorld.avsc

22:15:24.166 [debug] new schema `playground.HelloWorld` stored with global id `1`

22:15:24.166 [debug] schema `playground.HelloWorld` will be always resolved from memory
schema `playground.HelloWorld' will be registered
Strech commented 1 year ago

Your fix was released as v0.26.0 🎉 Thanks for your contribution!