Should await asyncpg.create_pool() return Pool instead of None | Pool

[sbdchd]

Thank you for the library!

After setting this up I found that my connection pool logic is being flagged

essentially I have:

connection_pool: asyncpg.Pool[asyncpg.Record]

def on_startup() -> None:
    global connection_pool
    connection_pool = await asyncpg.create_pool()

and I get an error:

Incompatible types in assignment (expression has type "Optional[Pool[Record]]", variable has type "Pool[Record]")  [assignment] mypy(error)

looking at Asyncpg's pool:

https://github.com/MagicStack/asyncpg/blob/40b16ea65f8b634a392e7e5b6509ee7dda45c4cd/asyncpg/pool.py#L978-L979

I think technically it can return None but it seems unlikely to happen for most use cases (maybe?)

so while Pool | None represents a possible case, I think it would be okay if it didn't include it in the types for better ergonomics

What do you think?

[bryanforbes]

I think you're right that await asyncpg.create_pool() will never produce None since create_pool() is returning a brand new Pool that won't have _initialize set, so awaiting the new Pool will produce itself. I imagine the best way to type this is to make create_pool() an async function so the typing for Pool.__await__ can still be correct. Does that sound like a good solution to you?

[bryanforbes]

By way of example, if create_pool() is typed as an async function, the following is the result:

import asyncpg

async def main() -> None:
    pool = await asyncpg.create_pool()
    reveal_type(pool)  # Pool[Record]
    reveal_type(await pool)  # Pool[Record] | None

[bryanforbes]

I changed the typing of asyncpg.create_pool() to an async function in e387d77 and made a new release (0.26.5)

[bryanforbes]

Evidently, I didn't take into account using asyncpg.Pool as a context manager, so I'm back to the drawing board on this. I reverted the change in 94eaa96 and released 0.26.6. This may come down to being an issue that has to be # type: ignore because the stubs are limited in what they can do. For now, you can pin to 0.26.5 (even though you won't be able to use create_pool() as a context manager), but I'll continue to think of a good solution for this problem.

[cdhinrichs]

I'm showing up late to the party. I've been down a rabbit hole lately because I seem to have a calling stack that awaits one time more than necessary. (Probably because I'm trying to use an @property pattern to work in a REPL.) At first I thought the None return type was a signal to try again, because the server was busy or something, so I put it in a loop and ran out of connections.

While I'm getting to the bottom of why I have the extra await in there, I have a workaround like so,

  async def _create_pool(self):
    self._pool = asyncpg.create_pool(**kwargs)
    await self._pool
    return self._pool

But I'm wondering, in case the pool is already initialized, how come it has to return None? Why not just return self if the case is so rare?

[cdhinrichs]

And now I see there's a fix in the latest version. Never mind then. For anyone tuning in, upgrade to the latest version.