guardian_db fails to compile with Ecto 3 rc (undefined function migrations_path/1) about guardian_db HOT 20 CLOSED

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

from guardian_db.

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.

from guardian_db.

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.

from guardian_db.

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

from guardian_db.

@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

from guardian_db.

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

from guardian_db.

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

from guardian_db.

That would be cool! thanks

from guardian_db.

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.

from guardian_db.

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

from guardian_db.

elixir-ecto/ecto#2763 (comment) Any ideas how to get this working?

from guardian_db.

Same fix in another library. Guess I can attempt a PR based on this.
kipcole9/money@c2766de

Still not sure how to best test this.

from guardian_db.

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

from guardian_db.

@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.

from guardian_db.

Blocked until Ecto 3.0 is released.

from guardian_db.

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.

from guardian_db.

@yordis Would you use the function_exported? check? #93 (comment)

from guardian_db.

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.

from guardian_db.

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

from guardian_db.

any news for the release?

from guardian_db.