elixir-inspector / ua_inspector

User agent parser library
Apache License 2.0
127 stars 23 forks source link

mix ua_inspector.download fails when using dynamic DB configuration #18

Closed smaximov closed 5 years ago

smaximov commented 5 years ago

I store the database in my app's priv directory. At first, I specified the database path as a path relative to the project root:

config :ua_inspector,
  database_path: "priv/ua_db"

It worked in development and test environments, but didn't work in releases. So I switched to dynamic configuration:

# config.exs
config :ua_inspector,
  init: {MyApp, :resolve_ua_inspector_db, ["ua_db"]}

MyApp.resolve_ua_inspector_db/1 is defined as follows:

# my_app.ex
defmodule MyApp do
  def resolve_ua_inspector_db(dbname) do
    dbpath = resolve_priv([dbname])

    Application.put_env(:ua_inspector, :database_path, dbpath)
  end

  defp resolve_priv(parts), do: Application.app_dir(:my_app, ["priv" | parts])
end

This works, but now when running mix ua_inspector.download, I get the following error:

** (UndefinedFunctionError) function MyApp.resolve_ua_inspector_db/1 is undefined (module MyApp is not available)
    MyApp.resolve_ua_inspector_db("ua_db")
    lib/mix/tasks/ua_inspector/download.ex:23: Mix.Tasks.UaInspector.Download.run/1
    (mix) lib/mix/task.ex:331: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:79: Mix.CLI.run_task/2

I figured the error happens because the mix task tries to use the code that is not loaded yet. I've tried it on versions 1.0.0 and 1.1.0.

smaximov commented 5 years ago

As a workaround, one can invoke the app.start mix task before running ua_inspector.download with mix do:

mix do app.start, ua_inspector.download

Another solution might be to load configured apps before invoking UAInspector.Config.init_env/0 in ua_inspector.download, this requires a new configuration option to be introduced:

config :ua_inspector,
  init: {MyApp, :resolve_ua_inspector_db, ["ua_db"]},
  load_apps: [:my_app]

Changes to the mix task:

--- /tmp/aaa.ex 2019-07-15 11:30:54.708207207 +0300
+++ /tmp/bbb.ex 2019-07-15 11:32:11.592203895 +0300
@@ -36,6 +36,12 @@
   ]

   def run(args) do
+    load_apps = Application.get_env(:ua_inspector, :load_apps, [])
+
+    for app <- load_apps do
+      :ok = Application.load(app)
+    end
+
     :ok = Config.init_env()

     {opts, _argv, _errors} = OptionParser.parse(args, @cli_options)
mneudert commented 5 years ago

It looks like just loading an application will not work, at least locally for me:

{:error, {'no such file or directory', 'ua_inspector_test.app'}}

What however did work was calling Mix.Task.run("app.start") before initializing the environment. That has the (yet to be solved) effect of trying to load the database files and log errors when there are none yet.

But to at least make the mix task work properly having a way to start without getting "database file not found" messages might be the most simple solution available.

mneudert commented 5 years ago

Could you try the change (https://github.com/elixir-inspector/ua_inspector/commit/23958cf474726c7c9172c33fe76f38784b2d30d8) I applied to the master branch to see if my previous assumption was really correct and this solves the problem?

smaximov commented 5 years ago

It looks like just loading an application will not work, at least locally for me

This one was just a guess, I didn't test it myself, sorry :) I should have made it clear.

Could you try the change (23958cf) I applied to the master branch to see if my previous assumption was really correct and this solves the problem?

Yes, it works!

mneudert commented 5 years ago

:heart: :blue_heart: :green_heart: :purple_heart: :yellow_heart:

The fix is now available in the newly release version 1.2.0.

doughsay commented 5 years ago

I just updated to 1.2.0. mix ua_inspector.download starts my entire application now, when it didn't before. Starting my entire application causes database and message broker connections to open, etc. This won't work for me when I'm building docker images and external services are not available. Is there no way to just download the file without requiring the application be loaded?

doughsay commented 5 years ago

I confirmed this breaks my builds; so I opened a new issue: https://github.com/elixir-inspector/ua_inspector/issues/19