oban-bg / oban

💎 Robust job processing in Elixir, backed by modern PostgreSQL and SQLite3
https://oban.pro
Apache License 2.0
3.29k stars 308 forks source link

Oban.Repo.one() typespec is too restrictive #796

Closed flupke closed 1 year ago

flupke commented 1 year ago

Environment

Oban Version: 2.13.2 PostgreSQL Version: 14.4 Elixir & Erlang/OTP Versions: 1.14.0 / OTP 25

Current Behavior

The following query:

defp wait_until_oban_queue_is_drained(name) do
  from(j in Oban.Job,
    select: count(j.id),
    where: j.queue == ^name and j.state not in ["completed", "discarded", "cancelled"]
  )
  |> then(&Oban.Repo.one(Oban.config(), &1))
  |> case do
    0 ->
      :ok

    count ->
      Logger.info("Waiting for #{count} jobs to complete in queue #{name}")
      Process.sleep(1000)
      wait_until_oban_queue_is_drained(name)
  end
end

Creates a dialyzer error:

The pattern can never match the type.

Pattern:
0

Type:
nil | %{atom() => _}

Because Oban.Repo.one() typespec is too restrictive:

  @doc "Wraps `c:Ecto.Repo.one/2`."
  @doc since: "2.2.0"
  @spec one(Config.t(), Queryable.t(), Keyword.t()) :: Schema.t() | nil
  def one(conf, queryable, opts \\ []) do
    with_dynamic_repo(
      conf,
      fn -> conf.repo.one(queryable, query_opts(conf, opts)) end
    )
  end

I think it should include term() in the return types union, to match the Ecto return type: https://hexdocs.pm/ecto/Ecto.Repo.html#c:one/2

Expected Behavior

This query should not produce a dialyzer error.

sorentwo commented 1 year ago

You're better off using your application's own Repo module for queries, even for Oban.Job. The Oban.Repo module is for internal modules like engines and plugins, and it's primarily a wrapper for dynamic repos.

That said, I'll look into fixing up the typespec(s).

sorentwo commented 1 year ago

@flupke I've just opened #798 to warn about this use case and sidestep the typespec issue.