Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require new pool tests #732

Closed
5 tasks done
josevalim opened this issue Jun 30, 2015 · 8 comments
Closed
5 tasks done

Require new pool tests #732

josevalim opened this issue Jun 30, 2015 · 8 comments

Comments

@josevalim
Copy link
Member

For @elixir-lang/ecto-adapters,

  • MySQL
  • Postgres
  • MSSQL
  • SQLite3
  • Mongo

Just do this: 16f27f6#diff-2c5b8dbebddda6206ddb2951ce5b1bc8R9

@jazzyb
Copy link
Contributor

jazzyb commented Jul 1, 2015

SQLite: jazzyb/sqlite_ecto@9e9bf3d

@mobileoverlord
Copy link
Contributor

MSSQL: livehelpnow/tds_ecto@9b19c28

@michalmuskala
Copy link
Member

Mongo: elixir-mongo/mongodb_ecto@7f44dbb

Also I had to change how I handle pools after changes in Mongo.Adapters.SQL, I noticed one thing that worries me. In start_link we are using pool that can be provided in options, but then in query always the default one is used. Wouldn't this cause issues?
start_link: https://github.com/elixir-lang/ecto/blob/master/lib/ecto/adapters/sql.ex#L355-363
query: https://github.com/elixir-lang/ecto/blob/master/lib/ecto/adapters/sql.ex#L180

@josevalim
Copy link
Member Author

Michal, btw, you don't need to use the Ecto.Adapters.SQL.Sandbox pool. Just
keep size as 1.

It won't cause issues because the idea is that if you are using custom
pools, you will override pool too.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

On Wed, Jul 1, 2015 at 9:07 PM, Michał Muskała notifications@github.com
wrote:

Mongo: elixir-mongo/mongodb_ecto@7f44dbb
elixir-mongo/mongodb_ecto@7f44dbb

Also I had to change how I handle pools after changes in
Mongo.Adapters.SQL, I noticed one thing that worries me. In start_link we
are using pool that can be provided in options, but then in query always
the default one is used. Wouldn't this cause issues?
start_link:
https://github.com/elixir-lang/ecto/blob/master/lib/ecto/adapters/sql.ex#L355-363
query:
https://github.com/elixir-lang/ecto/blob/master/lib/ecto/adapters/sql.ex#L180


Reply to this email directly or view it on GitHub
#732 (comment).

@michalmuskala
Copy link
Member

I reverted the change with the sandbox pool elixir-mongo/mongodb_ecto@ac58a65

@josevalim
Copy link
Member Author

@michalmuskala I also see that you are defining a new pool just because of those tests, I will try to change it so it is not required.

@josevalim
Copy link
Member Author

@michalmuskala i have pushed a commit to master so you no longer need to define a pool only for those tests, they all use test repo now!

@michalmuskala
Copy link
Member

I updated the tests: elixir-mongo/mongodb_ecto@f6da6ff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants