Replace local names with registry #331
Conversation
This retains the previous behaviour.
This effectively means that name can be an arbitrary term, such as `{:tenant, tenant_id}`.
This increases the chances of catching some unintended module.concat, atom manipulation, and other bugs where the internals assume something about the name type.
There was a problem hiding this comment.
Thanks for this amazing work. The global registry, storing config, using the name as the id automatically, and the dramatically improved isolation guarantees are brilliant.
I've left some nitpicks, nearly all of which are in the name of consistency and to satisfy my neurosis. This is a great start to a v2.2.0 🎉
| @@ -12,33 +12,33 @@ defmodule Oban.Integration.ControllingTest do | |||
| end | |||
|
|
|||
| test "starting individual queues dynamically" do | |||
| start_supervised_oban!(queues: [alpha: 10]) | |||
| name = start_supervised_oban!(queues: [alpha: 10]).name | |||
There was a problem hiding this comment.
The look of this may take me some getting used to. The fact that it is a unique reference for every test is fantastic though.
There was a problem hiding this comment.
Hopefully it's a bit simpler now, after I applied your proposal of only returning the name.
| multi = Multi.new() | ||
| multi = Oban.insert(context.name, multi, :job_1, UniqueWorker.new(%{id: 1})) | ||
| multi = Oban.insert(context.name, multi, :job_2, UniqueWorker.new(%{id: 2})) | ||
| multi = Oban.insert(context.name, multi, :job_3, UniqueWorker.new(%{id: 1})) | ||
| assert {:ok, %{job_1: job_1, job_2: job_2, job_3: job_3}} = Repo.transaction(multi) |
There was a problem hiding this comment.
What a shame, this was a nice pipe chain 😢
There was a problem hiding this comment.
What can I say. You win some, you lose some :-)
|
I think I addressed everything. Let me know if something else is needed. |
|
That was everything. This is exciting stuff, puno hvala! 💛 |
This replaces locally registered names with Registry based names, which allows us to use arbitrary terms as oban instance names, e.g.
{Oban, name: {:tenant, 1}}. This is the preparation for supporting running dynamic Oban instances which can work with dynamic Ecto repos, as discussed in #326.In terms of function signatures, the change should be backwards compatible. However, if the client code relied on the previous name registration, it will break.
Summary of changes:
Oban.child_spec.