Sync connector 225 #237
Sync connector 225 #237
Conversation
ewjoachim
force-pushed
the
sync-connector-225
branch
from
7db8821 to
0016349
Compare
|
Ok, I've done a 2nd pass. |
ewjoachim
force-pushed
the
sync-connector-225
branch
from
0016349 to
c6232f1
Compare
ewjoachim
force-pushed
the
sync-connector-225
branch
4 times, most recently
from
728ac84 to
f9ce79c
Compare
ewjoachim
requested review from
CorBott,
DainDwarf,
mgu,
pmourlanne and
sophie-ulti
as code owners
Codecov Report
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 24 25 +1
Lines 1100 1226 +126
Branches 135 140 +5
==========================================
+ Hits 1100 1226 +126
Continue to review full report at Codecov.
|
docs/discussions.rst
Outdated
| This mode is necessary in multi-threaded cases. | ||
| - "mixed" I/O (synchronously launching an event loop, and have asynchronous coroutine | ||
| run under the hood). | ||
| This mode will use less connections when used within another job. |
There was a problem hiding this comment.
I've been thinking about this whole thing over the week-end 😄 And I must say that I am not a big fan of supporting two ways of doing synchronous I/O. The paragraph above, where we explain that there's a "classic" way and a "mixed" way, is, to me, a clear sign of complexity and poor design. I think what I'd like to see a very clear separation between the sync API and the async API. I'd even consider removing the add_sync_api decorator, and making the async API async only. And introducing a separate sync API for sync applications. I know my words may sound a bit theoretic… but that's all I have at this point.
There was a problem hiding this comment.
Ok, but what about deferring from within a task ? You already have an app and you most probably wish not to make a new pool for this.
ewjoachim
force-pushed
the
sync-connector-225
branch
from
7ea5973 to
d4e896e
Compare
elemoine
force-pushed
the
sync-connector-225
branch
2 times, most recently
from
f30697c to
d812171
Compare
docs/discussions.rst
Outdated
| This mode is necessary in multi-threaded cases. | ||
| - "mixed" I/O (synchronously launching an event loop, and have asynchronous coroutine | ||
| run under the hood). This mode is adapted for synchronously defering jobs from other | ||
| job, from within the workers. |
There was a problem hiding this comment.
| job, from within the workers. | |
| jobs, from within the workers. |
docs/discussions.rst
Outdated
| - "classic" synchronous I/O (using synchronous database drivers such as ``Psycopg2``). | ||
| This mode is necessary in multi-threaded cases. | ||
| - "mixed" I/O (synchronously launching an event loop, and have asynchronous coroutine | ||
| run under the hood). This mode is adapted for synchronously defering jobs from other |
There was a problem hiding this comment.
| run under the hood). This mode is adapted for synchronously defering jobs from other | |
| run under the hood). This mode is adapted for synchronously deferring jobs from other |
docs/discussions.rst
Outdated
| @@ -265,11 +292,15 @@ this? Here are a few resources: | |||
| with sync **and** async callers: "Just add await" from Andrew Godwin: | |||
| https://www.youtube.com/watch?v=oMHrDy62kgE | |||
|
|
|||
| That being said, synchronous defers (see `discussion-sync-defer`) rely on a few | |||
| specific methods that have been duplicated by hand (one async and one sync version). | |||
There was a problem hiding this comment.
@ewjoachim is this comment sill valid? I am wondering about "duplicated by hand".
There was a problem hiding this comment.
I've clarified it, but it's still valid.
ewjoachim
force-pushed
the
sync-connector-225
branch
from
d812171 to
756f4fd
Compare
Closes #225
Haven't touched the tests yet.
Successful PR Checklist: