guardian_db fails to compile with Ecto 3 rc (undefined function migrations_path/1)

[nathany]

See https://github.com/elixir-ecto/ecto/issues/2763

== Compilation error in file lib/mix/tasks/guardian_db.gen.migration.ex ==
** (CompileError) lib/mix/tasks/guardian_db.gen.migration.ex:21: undefined function migrations_path/1
    (stdlib) lists.erl:1338: :lists.foreach/2
    (stdlib) erl_eval.erl:680: :erl_eval.do_apply/6

I'm in the midst of updating a Phoenix app to use Ecto 3 (and ecto_sql), but ran into this compile failure when updating guardian_db from 0.8 to 1.1.

https://github.com/ueberauth/guardian_db/blob/master/lib/mix/tasks/guardian_db.gen.migration.ex#L21

[nathany]

Guess I'll do a PR to use Ecto.Migrator.migration_path.

[hassox]

That would be cool! thanks

[nathany]

No worky in Ecto 2.2

warning: function Ecto.Migrator.migrations_path/1 is undefined or private. Did you mean one of:

      * migrations/2

  lib/mix/tasks/guardian_db.gen.migration.ex:21

I'm also not sure the best way to test this. Back in the Rails days we had multiple Gemfiles so CI could test ecto_sql 3 and ecto 2.2.

[nathany]

@hassox I'm not sure how to proceed. Not an Elixir guru. Apologies.

[nathany]

https://github.com/elixir-ecto/ecto/issues/2763#issuecomment-433186337 Any ideas how to get this working?

[nathany]

Same fix in another library. Guess I can attempt a PR based on this. https://github.com/kipcole9/money/commit/c2766dece7f241f25b86999876d756d7b7d7a4be

Still not sure how to best test this.

[hassox]

Not sure of the best path but a path forward could be:

def migrations_path(repo) do
  if function_exported?(Ecto.Migrator, :migrations_path, 1) do
    Ecto.Migrator.migrations_path(repo)
  else
    Mix.Ecto.migrations_path(repo)
  end
end

If this were in runtime code I'd look for a compile time solution but since this is a mix script I think it should be ok.

[nathany]

Btw, I've sometimes been seeing a strange error when running the guardian_db tests. Any ideas?

* creating priv/temp/guardian_db_test/migrations
* creating priv/temp/guardian_db_test/migrations/20181025213144_guardiandb.exs
** (EXIT from #PID<0.90.0>) an exception was raised:
    ** (UndefinedFunctionError) function Ecto.Adapters.Postgres.child_spec/2 is undefined or private
        (ecto) Ecto.Adapters.Postgres.child_spec(Guardian.DB.Test.Repo, [otp_app: :guardian_db, repo: Guardian.DB.Test.Repo, timeout: 15000, pool_timeout: 5000, adapter: Ecto.Adapters.Postgres, database: "guardian_db_test", pool: Ecto.Adapters.SQL.Sandbox, priv: "priv/temp/guardian_db_test", pool_size: 1])
        (ecto) lib/ecto/repo/supervisor.ex:123: Ecto.Repo.Supervisor.init/1
        (stdlib) supervisor.erl:295: :supervisor.init/1
        (stdlib) gen_server.erl:374: :gen_server.init_it/2
        (stdlib) gen_server.erl:342: :gen_server.init_it/6
        (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

[nathany]

@doomspork I apologize for my poor online communication skills and the misunderstanding that resulted on #94.

I spent an afternoon working on that pull request to the best of my ability (I am new to Elixir, but had a lot of help from many people). It even incorporated the TravisCI changes requested in #90 by comparing to your other travis file.

Thank you for providing this library and supporting it. I'll try to do better in future open source interactions -- in what I say and don't say. Take care.

[yordis]

Blocked until Ecto 3.0 is released.

[nathany]

Last week I made the mistake of working on a pull request before asking enough questions to fully understand what the maintainers of this project wanted. Instead I made assumptions based on how I've seen multiple versions of a library supported in the Ruby on Rails ecosystem, and was excited to learn that it was possible to have multiple lockfiles in Elixir too.

Based on the comment from @yordis, is the plan to have a new release of guardian_db that requires ecto 3.0, and have people using ecto 2.x use an older version of guardian_db? If so, that sounds like a fine plan, and much simpler than my approach.

If instead you want to support multiple versions of ecto simultaneously, I'm curious how you would do it?

If anyone else looking at this issue is upgrading to ecto 3.0 rc, feel free to use my fork until ecto 3 is released, and a permanent solution is available.

{:guardian_db, "~> 1.1.0", github: "aai/guardian_db", branch: "migrations"},

FWIW, I never asked anyone to merge my pull request faster. My hope was that someone could use my work as a starting point if I wasn't able to continue working on it, as I'm often pulled from project to project.

Reading through my comments in #94, I recognize that I came across as an arrogant jerk, and for that I apologize.

It was a frustrating day or two, with 258 test failures upgrading guardian from 0.14 to 1.1 (thankfully now resolved). Then amplified by my open source efforts being criticized and disregarded without clear direction on how to do it better. Still, it's on me for not taking a step back and reacting in a calm manner. Sorry again, and thank you again for all your work on this library.

[idyll]

Blocked until Ecto 3.0 is released.

Ecto 3.0 dropped about 5 hours ago:

https://github.com/elixir-ecto/ecto/releases/tag/v3.0.0

[yordis]

Based on the comment from @yordis, is the plan to have a new release of guardian_db that requires Ecto 3.0, and have people using ecto 2.x use an older version of guardian_db? If so, that sounds like a fine plan, and much simpler than my approach.

@nathany yes that is correct.

As we are a small team and maintaining backward compatibility on because Ecto itself means that all the issue related to it will become part of the source code and our responsibility to maintain.

Instead I made assumptions based on how I've seen multiple versions of a library supported in the Ruby on Rails ecosystem

This is a rabbit hole that nobody likes, and to be honest it only contributes to the culture of users to expect core contributors to keep their application working because they do not take the time to upgrade.

The new release will require Ecto 3 only, people are not forced to upgrade and probably their application works in the current version.

If instead you want to support multiple versions of ecto simultaneously, I'm curious how you would do it?

If they decide to upgrade then they will need to take responsibilities on do so.

Otherwise, just let the current version as it is right now.

[yordis]

Reading through my comments in #94, I recognize that I came across as an arrogant jerk, and for that I apologize.

You live you learn, that is the past, focus on the future.

@nathany if you want to work on this and update your branch, just fix the breaking changes that Ecto introduced related to migrations_path/1

[nathany]

@yordis Would you use the function_exported? check? https://github.com/ueberauth/guardian_db/issues/93#issuecomment-433189687

[yordis]

@nathany use Ecto.Migrator.migrations_path/1 https://github.com/elixir-ecto/ecto_sql/blob/8f4104fec119d6bd2a3080660ddea099bce5ffeb/lib/ecto/migrator.ex#L32

Since this is the actual function on Ecto >3, we will need to add ecto_sql as a dependency since we need it as well.

Don't check for Mix.Ecto.migrations_path/1 since that was related to Ecto <3

[nathany]

ecto_sql 3.0.0 final is out today. I've amended @tanweerdev's commit to use it and opened PR #99.

I don't know exactly how your release process works. Will this be guardian_db @version "2.0.0"?

Thank you.

[yordis]

@nathany when it comes to changing the versioning we do that, but yes, we will use v2 since it has a breaking change.

[nathany]

Thanks @yordis. We're using {:guardian_db, "~> 2.0.0", github: "ueberauth/guardian_db"}, until such time as 2.0 is released.

[hykw]

any news for the release?