Async iterators payload #2873

asvetlov · 2018-03-24T14:11:24Z

Implementation of #2802

asvetlov · 2018-03-24T14:12:02Z

Working on docs

codecov-io · 2018-03-24T19:28:01Z

Codecov Report

Merging #2873 into 3.1 will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              3.1    #2873      +/-   ##
==========================================
+ Coverage   97.97%   97.98%   +<.01%     
==========================================
  Files          39       39              
  Lines        7464     7499      +35     
  Branches     1310     1316       +6     
==========================================
+ Hits         7313     7348      +35     
  Misses         48       48              
  Partials      103      103

Impacted Files	Coverage Δ
aiohttp/payload.py	`98.95% <100%> (+0.21%)`	⬆️
aiohttp/multipart.py	`95.47% <100%> (ø)`	⬆️
aiohttp/payload_streamer.py	`100% <100%> (ø)`	⬆️

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 77623f8...5caa91d. Read the comment docs.

asvetlov · 2018-03-25T12:42:28Z

Ready for review

kxepal · 2018-03-25T14:20:15Z

aiohttp/payload.py

+class AsyncIterablePayload(Payload):
+
+    def __init__(self, value, *args, **kwargs):
+        assert isinstance(value, AsyncIterable), \


Shouldn't here be a regular if not isinstace: raise TypeError check?

It should probably but other classes in the module use assert statements.

kxepal · 2018-03-25T14:23:17Z

aiohttp/payload.py

 def get_payload(data, *args, **kwargs):
    return PAYLOAD_REGISTRY.get(data, *args, **kwargs)


-def register_payload(factory, type):
+def register_payload(factory, type, *, order=Order.normal):


order is unused here. Any reasons to pass it there?

kxepal · 2018-03-25T14:29:49Z

aiohttp/payload.py


-    def get(self, data, *args, **kwargs):
+    def get(self, data, _CHAIN=chain, *args, **kwargs):


Shouln't _CHAIN be keyword-only argument? Otherwise it will be overrided by a second argument which should get into args.

kxepal · 2018-03-25T14:31:10Z

aiohttp/payload.py

            if isinstance(data, type):
                return factory(data, *args, **kwargs)

        raise LookupError()

-    def register(self, factory, type):
-        self._registry.append((factory, type))
+    def register(self, factory, type, order=Order.normal):


Everywhere else order is keyword-only argument. Shouldn't it be there the same for consistency?

kxepal · 2018-03-25T14:33:14Z

aiohttp/payload.py

+
+    async def write(self, writer):
+        try:
+            while True:


Here need a check that self._iter is not None for the case when the same instance of this class will be used twice.

kxepal · 2018-03-25T14:34:07Z

aiohttp/payload.py

+    async def write(self, writer):
+        try:
+            while True:
+                chunck = await self._iter.__anext__()


s/chunck/chunk/

asvetlov · 2018-03-25T14:37:38Z

Fixed everything except assert statements.

Working on asserts.

Thanks for review

kxepal

LGFM

asvetlov · 2018-03-26T06:39:19Z

I've reverted the change for:

Here needs a check that self._iter is not None for the case when the same instance of this class will be used twice.

Don't want to support such trick case. If an instance of the class will be used twice -- this is not intended behavior, better to fail in the case.
Failing fast is even better but I not sure if we really need to add extra checks. Anyway, passing the same generator instance twice into itertools API leads to unpredictable behavior too without any protection. Why should aiohttp be stricter?

kxepal · 2018-03-26T08:08:49Z

@asvetlov
No, I didn't ask for support. I was about to throw RuntimeError or sort of explicitly about that case instead of having weird AttributeError: 'NoneType' object has no attribute '__anext__'.

asvetlov · 2018-03-26T14:24:27Z

Done

asvetlov · 2018-03-26T14:25:43Z

async generators have an internal check for the case but I want to support any async iterable object (which could have no such strict checking).

lock · 2019-10-28T07:03:51Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

asvetlov added 2 commits March 24, 2018 20:27

Add try_first/try_last options to payload registration methods

41b23ff

Add tests

20e1c17

asvetlov changed the title ~~Async iterators payload~~ [WIP] Async iterators payload Mar 24, 2018

Fix python 3.5

d257643

asvetlov added 2 commits March 25, 2018 18:48

Document async iterator

2c3f3fb

Deprecate streamer

32cc8c1

asvetlov changed the base branch from master to 3.1 March 25, 2018 12:41

asvetlov changed the title ~~[WIP] Async iterators payload~~ Async iterators payload Mar 25, 2018

kxepal reviewed Mar 25, 2018

View reviewed changes

Pass missing parameter

c1c6ba3

kxepal reviewed Mar 25, 2018

View reviewed changes

Fix notes

9ab9608

Convert assertions into TypeErrors

33177c3

kxepal approved these changes Mar 25, 2018

View reviewed changes

Tune tests

5caa91d

asvetlov merged commit 9f2de0d into 3.1 Mar 27, 2018

asvetlov deleted the async-payload branch March 27, 2018 12:39

asvetlov mentioned this pull request Mar 27, 2018

Support async iterators for data uploading on client #2802

Closed

lock bot added the outdated label Oct 28, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Async iterators payload #2873

Async iterators payload #2873

asvetlov commented Mar 24, 2018

asvetlov commented Mar 24, 2018

codecov-io commented Mar 24, 2018 •

edited

Loading

asvetlov commented Mar 25, 2018

kxepal Mar 25, 2018

asvetlov Mar 25, 2018

kxepal Mar 25, 2018

kxepal Mar 25, 2018

kxepal Mar 25, 2018

asvetlov Mar 25, 2018

kxepal Mar 25, 2018

asvetlov Mar 25, 2018

kxepal Mar 25, 2018

asvetlov Mar 25, 2018

kxepal Mar 25, 2018

asvetlov Mar 25, 2018

asvetlov commented Mar 25, 2018

kxepal left a comment

asvetlov commented Mar 26, 2018

kxepal commented Mar 26, 2018

asvetlov commented Mar 26, 2018

asvetlov commented Mar 26, 2018

lock bot commented Oct 28, 2019


		def get(self, data, args, *kwargs):
		def get(self, data, _CHAIN=chain, args, *kwargs):

Async iterators payload #2873

Async iterators payload #2873

Conversation

asvetlov commented Mar 24, 2018

asvetlov commented Mar 24, 2018

codecov-io commented Mar 24, 2018 • edited Loading

Codecov Report

asvetlov commented Mar 25, 2018

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

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 Mar 25, 2018

kxepal left a comment

Choose a reason for hiding this comment

asvetlov commented Mar 26, 2018

kxepal commented Mar 26, 2018

asvetlov commented Mar 26, 2018

asvetlov commented Mar 26, 2018

lock bot commented Oct 28, 2019

codecov-io commented Mar 24, 2018 •

edited

Loading