[PROPOSAL] Allow defoverridable for Ecto.Repo query api functions

[Sean Culver]

Hello!

I want to propose that we allow users to override the following methods (all, one, update, etc...) https://github.com/elixir-ecto/ecto/blob/04dd5bc597147e58375713d86c93fcf91756aa76/lib/ecto/repo.ex#L162-L249

at https://github.com/elixir-ecto/ecto/blob/04dd5bc597147e58375713d86c93fcf91756aa76/lib/ecto/repo.ex#L251

The major reason for doing this is to allow for people to have default attributes for each of those query methods. This specifically resolves an issue regarding prefixes and being able to set a "Global" prefix after load time.  

defmodule MyApp.Repo do 

  use Ecto.Repo, otp_app: :my_app

  def one(queryable, opts \\ []) do 

    opts = [prefix: "Any prefix after compile & boot/load time here"] # Subdomains are a good example of this, when you can only get the prefix from the conn.

    super(queryable, opts)

  end 

end

Thanks for taking a look at this! Thoughts?

I'd be happy to put together a PR with the implementation for this. (really is a one line change anyways)

Cheers!

Sean 

[Sean Culver]

Just thought of it, or it could be configurable, you could set a config List of overridable function, that way the default behavior is still the same.

[José Valim]

We don't want to encourage this kind of behaviour. You should prefer to compose by having a repo that wraps the other or by adding new functions. If you really want to do it, you can call defoverridable yourself.

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/7d0fa892-6b05-46da-bab3-5394f10c0905%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Sean Culver]

Thanks for the feedback and heads up. My current implementation is actually just wrapping the repo, setting defaults and delegating to the original repo manually. It just felt wrong though, and I was browsing defdelegate and came across defoverride in the docs, coming from a Ruby background it seemed like a cleaner way but alas I have to keep my Elixir hat on.

Thanks again for taking a look and the feedback.

Cheers,

Sean

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.

[Hisa Ishibashi]

Hi José,

I just joined the group because I was planning to make this same proposal. I thought of it while working on this PR for ExAudit to support ecto ~> 3.4. I noticed that the implementation for default_options/1 is defined as overridable and thought that maybe after these years you might have changed your mind on this proposal.

ExAudit supports audit of assocs in a very clever way, by relying on the children traversal already implemented in Ecto.Repo.Schema.

And by the way, why is are the opts passed to @callback implementations not passed to children? I noticed this while working on the mentioned PR. For example, I was trying something like Repo.insert(struct, audit: true) and noticed that the option was filtered in the function:

defp assoc_opts(_assocs, opts) do

Keyword.take(opts, [:timeout, :log, :telemetry_event, :prefix])

end

Stay safe and thanks for your contributions,

Regards,

Hisa

To unsubscribe from this group and stop receiving emails from it, send an email to elixi...@googlegroups.com.

[José Valim]

We have added both prepare_query and default_options with the purpose of being overridable. It is not a general behavior though.

[Hisa Ishibashi]

No problem, luckily I already did the deboverridable myself before reading this thread hehe.

So let me know what you think about passing all options given to Repo @callback implementations .e.g., insert/2, update/2, to children instead instead of the current assoc_opts. Or should I create a new post here or a GitHub issue?