Add multipart coromethod to web request handler #1074
Conversation
|
cc @yinzuojie |
Current coverage is 98.40% (diff: 100%)@@ master #1074 diff @@
==========================================
Files 28 28
Lines 6496 6501 +5
Methods 0 0
Messages 0 0
Branches 1091 1092 +1
==========================================
+ Hits 6392 6397 +5
Misses 54 54
Partials 50 50
|
|
We need an usage example in |
| def multipart(self, *, reader=multipart.MultipartReader): | ||
| """Return async iterator to process BODY as multipart.""" | ||
| if not self.content_type.startswith('multipart/'): | ||
| warnings.warn('Attempt to process multipart request with' |
There was a problem hiding this comment.
Is it possible to use .multipart() for application/x-www-form-urlencoded data?
There was a problem hiding this comment.
application/x-www-form-urlencoded - is about key=value&anotherkey=anothervalue format. It's not multipart thing. You probably mean multipart/form-data which is used to transfer binary data like files.
From the spec:
This attribute specifies the content type used to submit the form to the server (when the value of method is "post"). The default value for this attribute is "application/x-www-form-urlencoded". The value "multipart/form-data" should be used in combination with the INPUT element, type="file".
There was a problem hiding this comment.
For application/x-www-form-urlencoded I think we can have just .form() which returns MultiDictProxy(MultiDict()) as like .post() and can be called multiple times.
There was a problem hiding this comment.
I mean if Content-Type is not started with multipart/ it's maybe not a warning but an exception?
There was a problem hiding this comment.
No, that's bad idea since content-type validation must be done before any read method call. Raising exception means we have double ctype validation. Also, people may use custom content-type for their requests like applicaiton/vnd+multipart or whatever which won't pass this check.
The reason of this warning is to notify that things may go wrong, but since you call that method we think that you know what are you doing. May be it's practically useless though.
|
@kxepal could you add a test for fixing coverage issue. |
|
@asvetlov Sorry, yes. And an example. I'll return to the issue in few days. |
|
thanks. I would like to have this PR in next release. |
|
@asvetlov Done, sorry for delay. It seems windows tests fails for connector due to IPv4 vs IPv6. Is that known issue? |
|
I've added broken test yesterday, will fix it today ASAP. |
|
Ok, thanks! |
|
I've fixed tests. |
|
Hmm, from first glance all your changes should be covered. |
|
@asvetlov Yes, there shouldn't be a coverage misses. 100% is a really good goal! |
|
Looks good! |
|
But I still don't see a way to process 2GB file without reading the whole content in memory. |
|
@asvetlov Hm...you scary me. read_chunk does respects that size parameter. And then all depends on what kind of request we process. If it provides content length we basically read from Same story we do for chunked transfer, but with a tiny buffer equals a single chunk size since we eventually can read more than we need in single shot, so we need the way to roll back. Did I miss something? |
|
Aah, sorry. |
|
Great! You just saved travis from test on transferring 2GiB of data ((: But I found that I'm not able to pass more than 8MiB of data over test client using generator as data emitter. No errors around and warning, just bunch of data get missed. Throwing all of them with single big blob of data works right. That's all strange. |
|
Finally. I was chasing wrong reason of failure |
|
Thanks! |
This fixes #1067