feat: An option to disable triggers validation #51
Conversation
|
Thanks for the contribution! Thinking about this: to it might be clearer about what's going on if the application just didn't add EctoWatch to the supervision tree in these cases? So like: children = [
... other children ...
if Mix.env != :test do
{EctoWatch,
repo: MyApp.Repo,
pub_sub: MyApp.PubSub,
watchers: [
{User, :inserted},
{User, :updated},
{User, :deleted},
{Package, :inserted},
{Package, :updated}
]}
else
:ignore
end
Or maybe there could be a function: def ecto_watch_child_spec do
if Mix.env != :test do
{EctoWatch,
repo: MyApp.Repo,
pub_sub: MyApp.PubSub,
watchers: [
{User, :inserted},
{User, :updated},
{User, :deleted},
{Package, :inserted},
{Package, :updated}
]}
else
:ignore
end
endBut also, I think sometimes people are going to want to test their |
Indeed, I was trying to find a middle ground that will allow Maybe it's just better to accept that the library won't run out-of-the-box in a default test environment. That said, one can always explicitly run it with I'll give it a spin and let you know how it went. |
Hrmmm, that made me think, TBH... One goal is for it to "just work" for people... I wonder if perhaps if |
That makes sense. Yet, I think the library as close to working out-of-the-box in tests as it gets. The rest of the settings involve some integration work. Depending on the approach you take, maybe https://github.com/ash-project/igniter could manage that integration step automatically. And quite ironically, the one thing that kinda gets in the way of tests, at least for my project, is that automatic trigger validation. But I'm not sure if it's feasible to toggle it based on
Indeed, that's the approach I'm taking: Using a separate module to spin up `EctoWatch`# config/test.exs
conifg(:myapp, MyApp.RepoWatcher, enabled: false)
defmodule MyApp.RepoWatcher do
use Supervisor
def start_link(args) do
name = Keyword.get(args, :name, __MODULE__)
enabled? = Keyword.get_lazy(args, :enabled?, &enabled?(name))
if enabled? do
Supervisor.start_link(__MODULE__, args, name: __MODULE__)
else
:ignore
end
end
def enabled?(module) do
:myapp
|> Application.get_env(module, [])
|> Keyword.get(:enabled, true)
end
@impl true
def init(_) do
children = [
{EctoWatch,
repo: Mindme.Repo,
pub_sub: Mindme.PubSub,
watchers: [
MySchema
]}
]
opts = [strategy: :one_for_one]
Supervisor.init(children, opts)
end
endWhile in tests it looks like this: defmodule MyApp.DataTest do
use MyApp.DataCase, async: false
setup do
start_supervised({MyApp.RepoWatcher, enabled?: true})
end
endThat said, I'm yet to try it out for real 😅 |
|
There is another valid reason to diable the notifications. |
The changes introduce an option that disables triggers validation
The motivation behind the option is aimed mainly at test environments. Validation is great, but it works in an asynchronous manner, which gets in the way of sandboxed repos (
Ecto.Adapters.SQL.Sandbox.mode(Repo, :manual))So, when running tests, you can hit the following error:
(DBConnection.OwnershipError) cannot find ownership process for #PID
From my understanding, that happens because the validator tries to return a database connection to the pool, but isn't authorized to do that. Mostly because it was spawned outside test cases.
The
validate_triggers?makes it possible to skip validation in tests and avoid the above error altogether.That said, please let me know if you have any other ideas to avoid the mentioned scenario. I'm more than willing to cooperate on this.
Also, thank you for such a nice library! 🙇