add support for "async with cursor_context()" #265
Conversation
Codecov Report
@@ Coverage Diff @@
## master #265 +/- ##
==========================================
+ Coverage 93.66% 93.95% +0.28%
==========================================
Files 25 25
Lines 3744 3787 +43
Branches 199 202 +3
==========================================
+ Hits 3507 3558 +51
+ Misses 199 191 -8
Partials 38 38
Continue to review full report at Codecov.
|
|
Good point, is it possible to hack something so we can support both: |
|
ya I'll work something up in the next few days |
|
@jettify ok, I think I've done it to have the current method backwards and forwards compatible. Getting it to work was tricky. Just tweaking the unittests now for code coverage. Let me know what you think. |
|
@jettify ping :) |
|
I'm using this branch in development and it works very well 👍 . |
aiopg/utils.py
Outdated
|
|
||
| def __iter__(self): | ||
| # This will get hit if you use "yield from pool.cursor()" | ||
| if PY_35: |
There was a problem hiding this comment.
Don't think we need this warning.
yield from pool.cursor() works pretty fine even on Python 3.6.
There was a problem hiding this comment.
this is just so they to start using async with pool.cursor() instead of with (yield from pool.cursor()) so that the cursor will be actually closed at the end of the context.
There was a problem hiding this comment.
I guess in the future because right now it doesn't actually wait for the connection to be closed.
| @@ -163,6 +163,10 @@ def __init__(self, pool, conn): | |||
| self._pool = pool | |||
| self._conn = conn | |||
|
|
|||
| @property | |||
There was a problem hiding this comment.
- simplifies changes to _PoolCursorContextManager - bumps up docker client to work with latest docker for mac - bumps psycopg2 to latest release for testing - fixes issue with dsn comparison
# Conflicts: # tests/test_connection.py
|
hmm, @asvetlov let me know what you think of this error: https://travis-ci.org/aio-libs/aiopg/jobs/250130251#L1360 |
|
hmm, this was supposed to have been fixed by 8ca948e |
aio-libs@2a72b60 67acbbdd4#diff-3416729d6745fac0ca3dd44b22a69068
|
ok, looks like you later refactored it in 2a72b60#diff-3416729d6745fac0ca3dd44b22a69068 however it looks like it doesn't work with the latest psycopg2, so I'll let you decide how you want to fix it |
|
|
||
| async def test_pool_cursor_await_context_manager(loop, pg_params): | ||
| async with aiopg.create_pool(loop=loop, **pg_params) as pool: | ||
| with (await pool.cursor()) as cursor: |
There was a problem hiding this comment.
Should the yield from form be tested also?
There was a problem hiding this comment.
I believe this is tested in test_close_running_cursor
# Conflicts: # requirements.txt
# Conflicts: # requirements.txt
|
@asvetlov ok everything is now passing, let me know if you'd like any other changes |
|
@asvetlov anything left? let's get this merged :) (after I fix this having broken after merging from master) |
# Conflicts: # .gitignore # aiopg/__init__.py # setup.py
|
I can take a look in a few days |
|
FYI failures not my fault |
# Conflicts: # aiopg/cursor.py # tests/pep492/test_async_await.py
|
|
resolves #264