Add transaction model aiopg #219

vir-mir · 2016-11-26T01:24:39Z

Todo task

jettify · 2016-11-26T09:43:03Z

Just for your information, you can put python3.5 tests into tests/pep492 folder to avoid syntax errors on older python versions.

codecov-io · 2016-11-28T16:46:50Z

Codecov Report

Merging #219 into master will increase coverage by 0.3%.
The diff coverage is 98.21%.

@@            Coverage Diff            @@
##           master     #219     +/-   ##
=========================================
+ Coverage   93.23%   93.53%   +0.3%     
=========================================
  Files          23       25      +2     
  Lines        3427     3648    +221     
  Branches      189      194      +5     
=========================================
+ Hits         3195     3412    +217     
- Misses        194      198      +4     
  Partials       38       38

Impacted Files	Coverage Δ
aiopg/utils.py	`82.99% <100%> (+1.79%)`	⬆️
tests/test_cursor.py	`100% <100%> (ø)`	⬆️
tests/pep492/test_async_transaction.py	`100% <100%> (ø)`
aiopg/cursor.py	`98.76% <100%> (+0.06%)`	⬆️
tests/test_transaction.py	`96.55% <96.55%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9382f02...96cf798. Read the comment docs.

asvetlov · 2016-11-28T20:05:09Z

@vir-mir would you add missing tests?

vir-mir · 2016-11-28T20:58:59Z

@asvetlov hi! Yes.
I will finish, remove from the WIP (Work in process) title.

vir-mir · 2016-11-29T14:00:56Z

@asvetlov @jettify Done

sorry, I did not write the documentation.
But I added exemples/transactio.py and exemples/transactio_oldstyle.py

vir-mir · 2016-12-02T11:16:57Z

LGTM?

asvetlov · 2016-12-02T12:32:20Z

please wait a bit

asvetlov

Transaction class is perfect but I think we should add method factories to cursor for easy usage like

async with cur.begin():
    ...

async with cur.begin_nested():
    ...

For these methods we should choose appropriate default isolation level -- I have strong opinion that most of library users don't know about isolation levels at all.
At least it was very hard question on interviews provided by me.

asvetlov · 2016-12-04T20:54:29Z

aiopg/transaction.py

+    __slots__ = ('_cur', '_is_begin', '_isolation', '_unique_id')
+
+    def __init__(self, cur, isolation_level: IsolationLevel,
+                 readonly: bool = False, deferrable: bool = False):


Drop spaces around =.
Looks like bool=False is the preferable notation for function annotations.

asvetlov · 2016-12-04T21:15:40Z

@vir-mir we should decide: support typing annotations for the whole project or not.
I like it (while infrastructure is not really ready yet from my perspective).

If we will decide to support -- we should add annotations to every public aiopg method and check it by mypy tool strictly.
If not -- please don't add not checked annotations at all.
Would you be a volunteer for the task?

jettify · 2016-12-04T21:18:30Z

Exactly my thoughts, should we support external file like yarl or something inplace like here?

jettify · 2016-12-04T21:23:01Z

I believe we will add annotation eventually anyway, but lets drop it from this PR and add them separately with mypy supervision

vir-mir · 2016-12-05T10:43:46Z

Hi.
I propose to put the default isolation Read Committed
I add begin and begin_nested probably tomorrow.

jettify · 2016-12-05T11:10:30Z

Read Committed is default for PG so it is reasonable choice.

vir-mir · 2016-12-07T19:50:28Z

@asvetlov I'm add factories in cursor and drop spaces around

example

async def begin(engine):
    async with engine.cursor() as cur:
        async with cur.begin():
            await cur.execute("insert into tbl values(1, 'data')")

        async with cur.begin():
            await cur.execute('select * from tbl')
            row = await cur.fetchall()
            assert row == [(22, 'read only'), (1, 'data'), ]


async def begin_nested(engine):
    async with engine.cursor() as cur:
        async with cur.begin_nested():
            await cur.execute("insert into tbl values(1, 'data')")

            try:
                async with cur.begin_nested():
                    await cur.execute("insert into tbl values(1/0, 'no data')")
            except psycopg2.DataError:
                    pass

            async with cur.begin_nested():
                await cur.execute("insert into tbl values(2, 'data')")

        async with cur.begin_nested():
            await cur.execute('select * from tbl')
            row = await cur.fetchall()
            assert row == [(22, 'read only'), (1, 'data'),  (2, 'data'), ]

vir-mir · 2016-12-14T14:13:21Z

resolve conflicts

@asvetlov any news?

drednout · 2017-07-31T06:16:52Z

Hello, everyone!
What is the actual status of this PR? Is there any chances to have transaction functionality for raw SQL in aiopg upstream?

vir-mir · 2017-11-29T15:07:15Z

Dear all!
I rebuilt, I had to do push a force. Sorry. I also pull the master and corrected all the comments.

@asvetlov @jettify review, please

asvetlov · 2017-12-02T13:22:40Z

aiopg/cursor.py

@@ -146,6 +147,16 @@ def callproc(self, procname, parameters=None, *, timeout=None):
        else:
            yield from self._conn._poll(waiter, timeout)

+    def begin(self):
+        return _TransactionBeginContextManager(self._transaction.begin())


Not covered

asvetlov · 2017-12-02T13:22:52Z

aiopg/cursor.py

+        return _TransactionBeginContextManager(self._transaction.begin())
+
+    def begin_nested(self):
+        if not self._transaction.is_begin:


Not called by tests

asvetlov · 2017-12-02T13:23:15Z

aiopg/utils.py

+
+        @asyncio.coroutine
+        def __aexit__(self, exc_type, exc_val, exc_tb):
+            if exc_type is not None:


Not covered by tests

asvetlov · 2017-12-04T12:42:21Z

Cool!
The code looks perfect but lacks documentation.

jettify · 2017-12-04T13:10:32Z

Lets do docs in separate PR, this one already large.

…

On Mon, Dec 4, 2017, 14:42 Andrew Svetlov ***@***.***> wrote: Cool! The code looks perfect but lacks documentation. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#219 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANoZwJMw51cxPYUbIVlK_y-2B3hHInrks5s8-iugaJpZM4K8xsx> .

asvetlov · 2017-12-04T13:17:15Z

Lets do docs in separate PR, this one already large.

Ok. @vir-mir please merge.

Documentation is a show stopper for next release.

vir-mir · 2017-12-04T17:44:03Z

@jettify @asvetlov I added code samples for the cursor.
docs of the class of transactions, I propose to pass in another PR

96cf798

vir-mir changed the title ~~WIP: add transaction model aiopg~~ Add transaction model aiopg Nov 29, 2016

jettify mentioned this pull request Dec 2, 2016

when a new release? #225

Closed

asvetlov requested changes Dec 4, 2016

View reviewed changes

vir-mir force-pushed the 197-add-transaction-aiopg branch from 9d17d08 to d825cb6 Compare November 29, 2017 14:54

jettify mentioned this pull request Dec 1, 2017

Call for aiopg maintainer #384

Open

asvetlov approved these changes Dec 1, 2017

View reviewed changes

jettify approved these changes Dec 2, 2017

View reviewed changes

asvetlov requested changes Dec 2, 2017

View reviewed changes

aio-libs#197 Add transaction model aiopg

c43c10f

vir-mir force-pushed the 197-add-transaction-aiopg branch from d825cb6 to c43c10f Compare December 4, 2017 09:39

add docs

96cf798

asvetlov approved these changes Dec 4, 2017

View reviewed changes

vir-mir merged commit d43f38e into aio-libs:master Dec 4, 2017

Pliner mentioned this pull request Dec 14, 2020

Why transaction support implemented only for SAConnection? #197

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add transaction model aiopg #219

Add transaction model aiopg #219

vir-mir commented Nov 26, 2016 •

edited

jettify commented Nov 26, 2016

codecov-io commented Nov 28, 2016 •

edited by codecov bot

asvetlov commented Nov 28, 2016

vir-mir commented Nov 28, 2016

vir-mir commented Nov 29, 2016

vir-mir commented Dec 2, 2016

asvetlov commented Dec 2, 2016

asvetlov left a comment

asvetlov Dec 4, 2016

vir-mir Dec 5, 2016

asvetlov commented Dec 4, 2016

jettify commented Dec 4, 2016

jettify commented Dec 4, 2016

vir-mir commented Dec 5, 2016

jettify commented Dec 5, 2016

vir-mir commented Dec 7, 2016 •

edited

vir-mir commented Dec 14, 2016

drednout commented Jul 31, 2017

vir-mir commented Nov 29, 2017

asvetlov Dec 2, 2017

vir-mir Dec 4, 2017

asvetlov Dec 2, 2017

vir-mir Dec 4, 2017

asvetlov Dec 2, 2017

vir-mir Dec 4, 2017

asvetlov commented Dec 4, 2017

jettify commented Dec 4, 2017 via email

asvetlov commented Dec 4, 2017 •

edited

vir-mir commented Dec 4, 2017

Add transaction model aiopg #219

Add transaction model aiopg #219

Conversation

vir-mir commented Nov 26, 2016 • edited

jettify commented Nov 26, 2016

codecov-io commented Nov 28, 2016 • edited by codecov bot

Codecov Report

asvetlov commented Nov 28, 2016

vir-mir commented Nov 28, 2016

vir-mir commented Nov 29, 2016

vir-mir commented Dec 2, 2016

asvetlov commented Dec 2, 2016

asvetlov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 4, 2016

jettify commented Dec 4, 2016

jettify commented Dec 4, 2016

vir-mir commented Dec 5, 2016

jettify commented Dec 5, 2016

vir-mir commented Dec 7, 2016 • edited

vir-mir commented Dec 14, 2016

drednout commented Jul 31, 2017

vir-mir commented Nov 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 4, 2017

jettify commented Dec 4, 2017 via email

asvetlov commented Dec 4, 2017 • edited

vir-mir commented Dec 4, 2017

vir-mir commented Nov 26, 2016 •

edited

codecov-io commented Nov 28, 2016 •

edited by codecov bot

vir-mir commented Dec 7, 2016 •

edited

asvetlov commented Dec 4, 2017 •

edited