Multiple cursors support #801

Pliner · 2021-03-21T17:28:14Z

What do these changes do?

Support of multiple cursors which was initially proposed in #548 to resolve #535 and #364 by @vir-mir. I've slightly modified the implementation and rebased on top of master.

Are there changes in behavior for the user?

Related issue number

#777, #535, #364, #548, #778

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes

codecov · 2021-03-21T17:34:06Z

Codecov Report

Merging #801 (49824c3) into master (2e6e0f5) will increase coverage by 0.24%.
The diff coverage is 94.85%.

@@            Coverage Diff             @@
##           master     #801      +/-   ##
==========================================
+ Coverage   93.20%   93.45%   +0.24%     
==========================================
  Files          13       13              
  Lines        1545     1542       -3     
  Branches      176      178       +2     
==========================================
+ Hits         1440     1441       +1     
+ Misses         76       72       -4     
  Partials       29       29

Impacted Files	Coverage Δ
aiopg/sa/result.py	`91.26% <62.50%> (-0.45%)`	⬇️
aiopg/utils.py	`85.61% <84.61%> (+3.61%)`	⬆️
aiopg/sa/connection.py	`87.30% <96.15%> (+0.97%)`	⬆️
aiopg/connection.py	`91.36% <100.00%> (-0.47%)`	⬇️
aiopg/pool.py	`100.00% <100.00%> (ø)`
aiopg/sa/engine.py	`100.00% <100.00%> (ø)`
aiopg/sa/transaction.py	`84.81% <100.00%> (+0.81%)`	⬆️
aiopg/transaction.py	`100.00% <100.00%> (ø)`
aiopg/cursor.py	`99.40% <0.00%> (-0.60%)`	⬇️

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 2e6e0f5...49824c3. Read the comment docs.

lgtm-com · 2021-03-21T17:54:43Z

This pull request fixes 1 alert when merging 48cde02 into 65ca604 - view on LGTM.com

fixed alerts:

1 for Wrong number of arguments in a call

lgtm-com · 2021-03-21T18:25:28Z

This pull request fixes 4 alerts when merging e11c6fb into 65ca604 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

Pliner · 2021-03-21T18:53:51Z

aiopg/pool.py

@@ -43,7 +43,7 @@ def __init__(self, dsn, minsize, maxsize, timeout, *,
            raise ValueError("maxsize should be not less than minsize")
        self._dsn = dsn
        self._minsize = minsize
-        self._loop = get_running_loop(kwargs.pop('loop', None) is not None)


I suppose we are ready to drop this

Pliner · 2021-03-21T18:55:03Z

aiopg/sa/engine.py

-        finally:
-            self._engine = None
-            self._conn = None
+        asyncio.ensure_future(self._conn.close(), loop=self._loop)


close is coroutine, so we have to await it somehow

Pliner · 2021-03-21T18:56:30Z

aiopg/utils.py

-        self._obj = None
+        try:
+            if asyncio.iscoroutinefunction(self._obj.close):
+                await self._obj.close()


obj.close could be a coroutine, for instance, in case of Engine.acquire.

lgtm-com · 2021-03-21T19:07:29Z

This pull request fixes 4 alerts when merging 5c4f92e into 65ca604 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

lgtm-com · 2021-03-22T07:30:53Z

This pull request fixes 4 alerts when merging 6b2db0c into 59671d3 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

lgtm-com · 2021-03-22T08:19:04Z

This pull request fixes 4 alerts when merging 164a47c into 59671d3 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

lgtm-com · 2021-03-22T10:07:08Z

This pull request fixes 4 alerts when merging 774fd83 into 2e6e0f5 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

lgtm-com · 2021-03-22T10:16:44Z

This pull request fixes 4 alerts when merging f44009b into 2e6e0f5 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

lgtm-com · 2021-03-22T10:31:00Z

This pull request fixes 4 alerts when merging 49824c3 into 2e6e0f5 - view on LGTM.com

fixed alerts:

1 for Comparison using is when operands support `__eq__`
1 for Unnecessary 'else' clause in loop
1 for Module is imported with 'import' and 'import from'
1 for Wrong number of arguments in a call

Pliner commented Mar 21, 2021

View reviewed changes

Pliner changed the title ~~Multiple cursors support and fix of rollback for closed connection~~ Multiple cursors support Mar 22, 2021

Pliner force-pushed the multiple-cursors-and-rollback-fixes branch from 164a47c to b804c88 Compare March 22, 2021 09:53

Multiple cursors support

774fd83

Pliner force-pushed the multiple-cursors-and-rollback-fixes branch from b804c88 to 774fd83 Compare March 22, 2021 09:56

Pliner marked this pull request as ready for review March 22, 2021 09:57

Drop irrelevant doc

f44009b

Actualize doc

49824c3

Pliner merged commit c71e17b into master Mar 22, 2021

Pliner deleted the multiple-cursors-and-rollback-fixes branch March 22, 2021 10:49

Pliner mentioned this pull request Mar 22, 2021

resolve issues 535 and 364 #548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple cursors support #801

Multiple cursors support #801

Pliner commented Mar 21, 2021 •

edited

Loading

codecov bot commented Mar 21, 2021 •

edited

Loading

lgtm-com bot commented Mar 21, 2021

lgtm-com bot commented Mar 21, 2021

Pliner Mar 21, 2021

Pliner Mar 21, 2021

Pliner Mar 21, 2021

lgtm-com bot commented Mar 21, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

Multiple cursors support #801

Multiple cursors support #801

Conversation

Pliner commented Mar 21, 2021 • edited Loading

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

codecov bot commented Mar 21, 2021 • edited Loading

Codecov Report

lgtm-com bot commented Mar 21, 2021

lgtm-com bot commented Mar 21, 2021

Pliner Mar 21, 2021

Choose a reason for hiding this comment

Pliner Mar 21, 2021

Choose a reason for hiding this comment

Pliner Mar 21, 2021

Choose a reason for hiding this comment

lgtm-com bot commented Mar 21, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

lgtm-com bot commented Mar 22, 2021

Pliner commented Mar 21, 2021 •

edited

Loading

codecov bot commented Mar 21, 2021 •

edited

Loading