Ecto 3 support

[abarr]

Hi,

I am working on a project that we wanted to upgrade to Ecto 3.0.1. I have made changes to Triplex so we can continue development.

I could not find any guidance for submitting Pull Requests so I have followed the general conventions )However, this is my first OSS submission).

I have not updated the README as I am not sure how you want to reference eth changes.

I also could not get the tests to run outside of a project but they have passed when I have added it as a dependency to our project.

So far all use cases we used are working as expected. If I find any issues I will update.

I hope this helps.

Andrew

[mgiacomini]

@abarr could you fix the tests? i will checkout this and try to run locally :)

[abarr]

@mgiacomini I am new to contributing to open source so please be patient ...

I had a look at the Travis log and it says that the tests are being run with a config of elixir 1.4.2. I had to upgrade Triplex to use elixir 1.6 so it can use Ecto 3. Are you able to change the Travis config?

[abarr]

I will also have a go at getting the tests running locally

[kapilpipaliya]

Triplex.create(tenant) can't run migrations, when inside Transaction. it says (invalid_schema_name) schema "foo" does not exist but It created schema first like CREATE SCHEMA "foo" []

If I run Triplex.create(tenant) outside transaction it work successfully.

you can test running: Repo.transaction(fn -> Triplex.create("foo") end)

[kapilpipaliya]

workaround is do not use Triplex.create("foo") in Transaction.

[mgiacomini]

workaround is do not use Triplex.create("foo") in Transaction.

Yeah. I think the postgres doesn't support creating schemas/databases inside transactions, because we need to tell where it will run the operations, so if the schema/database doesn't exists it's not able to decide it.

[kapilpipaliya]

@mgiacomini but Repo.transaction(fn -> Triplex.create("foo") end) work with Ecto 2, so its already supported.

[mgiacomini]

@kapilpipaliya humm, so I need a close look into this.

[dhonig]

Any chance of this getting merged? I've just started a project using Ecto 3 and @abarr's work and would like to see Ecto 3 support added.

[churcho]

Any chance on getting this track resolved? I am getting

(DBConnection.ConnectionError) connection not available and request was dropped from queue
 after 970ms. You can configure how long requests wait in the queue using 
:queue_target and :queue_interval` 

when I run Triplex tests on @abarr's branch as well.

[coveralls]

Coverage increased (+10.5%) to 99.507% when pulling e62ceb20ff4c92ecd3ad9bcadee8f29e2f1edc6b on abarr:ecto_3_support into f49c43fc41169818a1454b3eb3ff5c3bc4815e4d on ateliware:master.

[kelvinst]

I got what's the problem with transaction: in Ecto 3 they changed the execution of migrations to run them on another process, and by default, when you run a transaction, it links the connection to the current process, and if you start another process inside of it, that process will not use the same connection, so everything done inside the other process will be in another transaction and therefore not get anything done by the original one.

I'm checking if there is a way to solve that.

[kelvinst]

@abarr I merged master to your branch and that broke the tests, so I did some changes to make the tests pass. You can review them on my commits for your branch.

@kapilpipaliya About the transaction problem, I explained on the previous comment. Created the issue https://github.com/elixir-ecto/ecto_sql/issues/84 for ecto people, since the only solution I see would have to add an option for Ecto.Migrator, as for how it is now, there is no way to run Triplex.create inside a transaction, so we will probably not release this any time soon, unfortunately.

[kelvinst]

So, José answered the issue and he would not like to change what we need on ecto, because it would add too much complexity there. So I had another idea, which will maybe solve https://github.com/ateliware/triplex/issues/47 too! Instead of running migrations when creating tenants, use a structure.sql dump file for it. It's a little bit more complex, but I guess it will pay off because it will make it a lot faster too.

[kelvinst]

@hauleth thanks for the review. I knew some of the functions are private API there. I switched the ones I could to other versions of it, but in some cases that's not possible because the public API is not the same, as for source_priv_dir for example, it returns the source code path, and Ecto.Migrator.migration_path returns it from inside the _build dir. I didn't dig into it if we could really change that, but as it is not the intention of this PR, I have created another issue for it: https://github.com/ateliware/triplex/issues/62

[hauleth]

@kelvinst you can check Mix.EctoSQL source and copy respective functions as these should use only public API of Ecto.

[kelvinst]

@kapilpipaliya and @abarr I had another idea on the transaction problem: we will make it clear we don't support that on the documentation, but will also give a way for users to guarantee the schema will get dropped if some of their stuff fails. Check https://github.com/ateliware/triplex/pull/61/commits/033d6d4c2b4ae6d33158d3f0300af36f6d0f6ed8 for details.

[kelvinst]

@kelvinst you can check Mix.EctoSQL source and copy respective functions as these should use only public API of Ecto.

Yep, I know, will do that on another PR though, since this is an old problem and there is already too much stuff on this PR.

[kelvinst]

@churcho can you test this PR again? Maybe it got solved by my fixes for it.

[kelvinst]

@hauleth at the end of the day I also did what you suggested, copied the code from Mix.EctoSQL.

This PR can be considered done now, I'm only doing some final touches for contributing and changelog files, and then I'll merge and release a new version!