samsondav/rihanna

Support Ecto Repo

lpil opened this issue · 9 comments

lpil commented

Hello!

Ecto has a nice sandbox that allows us to run database hitting tests concurrently when using Ecto's connection pool.

It'd be nice to be able to use this with Rihanna. Currently the library is hard-coded to use a dedicated Postgrex connection, however if the user was able to pass a connection the user could pass an Ecto repo and thus make use of the sandbox.

This would also remove the need for the Rihanna.Job.Postgrex in many cases, halving the number of database connections that Rihanna uses. Configuration could be added to prevent this process from being started if desired.

In addition it would also enable the user to enqueue more jobs and perform business login with the same SQL transaction as the connection will be shared across both automatically with Ecto, enabling a little more safety.

Here's some prior art on adding Ecto integration to a SQL executing lib without coupling tightly lpil/yesql@e692828

I will implement this feature if given the thumbs up.

Cheers,
Louis

lpil commented

It could be specified via mix config, as with the other configuration in Rihanna.

By default it continues to behave as it does today, however this config could be specified like so:

# A named postgrex connection is used
config :rihanna,
  enqueue_postgres_connection: {Postgrex, :my_postgrex_connection}
# An Ecto connection pool is used
config :rihanna,
  enqueue_postgres_connection: {Ecto, My.Repo}

In either of the above cases the Rihanna.Job.Postgrex process is not started.

Interesting idea. From what I understand, this is only for the process that enqueues jobs, is that correct?

lpil commented

What do you mean? :)

I mean, it's for Rihanna.enqueue - not the dispatcher?

lpil commented

Yes, it's only for enqueuing.

Ecto doesn't offer a way to always request the same connection from the pool so the dispatcher so the advisory locks wouldn't work. I believe it is possible to check out a connection long term with Ecto, but this is effectively the same as having a dedicated Postgrex connection so I'm not sure I see an advantage.

This seems like a good idea. Feel free to implement :)

lpil commented

Do you like the config design above?

Yeah it looks ok.

lpil commented

It could be passed at runtime for sure.

I suggested that API because Rihanna doesn't currently accept configuration beyond the Postgrex config at runtime so I thought that would be more in keeping with the current API.