python-trio / triopg

PostgreSQL client for Trio based on asyncpg
Other
45 stars 8 forks source link

triopg improvements #10

Closed shamrin closed 3 years ago

shamrin commented 3 years ago

triopg is a very nice library. It was a pleasure using it.

However, there were a couple of places I got stuck when I started:

  1. It was not clear that triopg wraps most (all?) of asyncpg functionality. I even started making my own tiny asyncpg wrapper before I've realized everything I need is available. Possible fixes: mention it in the README; write documentation.

(see PR #12)

  1. It was not clear how to use LISTEN functionality. Asking on gitter helped though. Possible fix: add simple listen helper to triopg:
    async with listen(conn, 'some.changes') as changes:
    async for change in changes:
        print('Postgres notification received:', change)

(see PR #13)

  1. connect does not sound very Trio-like. Possible fix: add open_connection or open_conn alias:

    async with triopg.open_connection(...) as connection:
    for row in await connection.fetch('select version()'):
        print(row['version'])
  2. It was not obvious how to write tests using real Postgres. I finally found that triopg own tests solve it pretty nicely. Possible fixes: document; provide pytest fixtures; send people to triopg own test source code.

  3. Types. Should be easily solvable once MagicStack/asyncpg/pull/577 lands.

Does it make sense to have (some of) them in triopg? I can try to work on them.

touilleMan commented 3 years ago

Hi @shamrin ,

  1. & 2. & 3.

The goal of triopg is to wrap all asyncpg features, while staying as close as possible to asyncpg api so that asyncpg's documentation stays relevant (the triopg documentation is basically it README, se we must keep things simple !). And as you discovered it's not that complicated to add wrappers around the add/remove methods ^^

I agree, a word about our ownconftest.py in the README could help a lot

5.

In theory it shouldn't be very complicated to copy the function signature typing in the asyncpg api and add it to triopg. I guess we would be able to add this once asyncpg's PR has been merged

Does it make sense to have (some of) them in triopg? I can try to work on them.

As you demonstrated, README definitly needs improvements, you're PRs are welcome :+1:

shamrin commented 3 years ago

Thank you for your response @touilleMan! I will try to improve the documentation.

(I still believe listen helper would be very useful. It was not at all obvious how to implement it using asyncpg API.)

touilleMan commented 3 years ago

implemented in #12 and #13 :smiley: