classification
Title: Support iterable bodies in httplib
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: catlee, davide.rizzo, georg.brandl, jhylton, merwok, orsenthil, pitrou, rcoyner, rhettinger, xuanji
Priority: normal Keywords: easy, needs review, patch

Created on 2008-06-30 18:01 by catlee, last changed 2011-01-15 18:27 by orsenthil. This issue is now closed.

Files
File name Uploaded Description Edit
python-3243.patch catlee, 2008-12-05 16:43 Patch for python2.7 review
issue_3243_py3k.diff xuanji, 2010-11-28 10:39
issue_3243_py3k_2.patch xuanji, 2010-11-29 15:53
issue_3243_py3k_3.patch xuanji, 2010-11-30 03:20
Issue3243-4.patch orsenthil, 2010-12-01 09:41
issue_3243_py3k_5.patch xuanji, 2010-12-01 15:49
Issue3243-6.patch orsenthil, 2010-12-02 03:14
issue_3243_py3k_6.patch xuanji, 2010-12-02 03:16
issue_3243_py3k_7.patch xuanji, 2010-12-02 03:28
Messages (61)
msg69014 - (view) Author: Chris AtLee (catlee) Date: 2008-06-30 18:01
httplib should support requests whose bodies are iterable objects.  This
would facilitate doing large file uploads via HTTP since you wouldn't
have to load the entire file into memory to create the request string.

Index: Lib/httplib.py
===================================================================
--- Lib/httplib.py      (revision 64600)
+++ Lib/httplib.py      (working copy)
@@ -688,7 +688,12 @@
        self.__state = _CS_IDLE

    def send(self, str):
-        """Send `str' to the server."""
+        """Send `str` to the server.
+
+        ``str`` can be a string object, a file-like object that supports
+        a .read() method, or an iterable object that supports a .next()
+        method.
+        """
        if self.sock is None:
            if self.auto_open:
                self.connect()
@@ -710,6 +715,10 @@
                while data:
                    self.sock.sendall(data)
                    data=str.read(blocksize)
+            elif hasattr(str,'next'):
+                if self.debuglevel > 0: print "sendIng an iterable"
+                for data in str:
+                    self.sock.sendall(data)
            else:
                self.sock.sendall(str)
        except socket.error, v:
msg77037 - (view) Author: Chris AtLee (catlee) Date: 2008-12-05 16:43
The attached patch implements this for python 2.7.  It also adds support
for iterable bodies in urllib2, where it is more generally useful. 
urllib2 enforces the presence of a Content-Length header in the request
if the body is an iterable, whereas httplib does not.

The patch also includes updates to docs and tests (which all pass as of
r67584 on my macbook)
msg84304 - (view) Author: Jeremy Hylton (jhylton) Date: 2009-03-28 12:21
Seems like a reasonable feature request.  I'm going to apply a variant
of the patch in 3.1 first.
msg100817 - (view) Author: Ryan Coyner (rcoyner) Date: 2010-03-11 00:02
This patch and its tests still work. Any particular reason why it hasn't been adopted yet?
msg110675 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-18 19:32
Could someone with knowledge of httplib please move this forward, all the comments I see are positive.
msg120999 - (view) Author: Éric Araujo (merwok) * (Python committer) Date: 2010-11-12 00:46
The patch needs to be ported to the py3k branch.
msg122647 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-28 10:39
Hi, attached is a ported version of the patch that applies against py3k.
msg122707 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2010-11-28 19:10
Instead of 
    hasattr(str,'next')

consider using
    isinstance(str, collections.Iterable)

Also consider changing the variable name from the now overly type specific, "str" to something like "source" to indicate the significance of the data, not its type.
msg122756 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-29 01:46
Hi Raymond, I assume you're referring to catlee's patch? 'str' has been changed to 'data' in python 3.2

While porting the patch I ran into this issue, which is that isinstance(str, collections.Iterable) doesn't behave exactly like hasattr(str,'next')


>>> hasattr("lol", '__next__')
False
>>> isinstance("lol", collections.Iterable)
True

so using the isinstance method would actually match against strings, which iirc would make the program fail. I can confirm later if needed.
msg122784 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-11-29 09:15
xuanji, the issue you stumbled upon was just fixed by Raymond for the report Issue10565.
msg122787 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-29 11:23
What a timely coincidence. I'll try out the change soon.
msg122819 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-29 15:53
Changed according to Raymond's suggestion. I realized that there are some classes (str, bytes, array.array) that are iterable but should not be handled by the iteration logic provided by catlee. eg: if we iterate through b'a message' we'd get a bunch of ints and sending them is wrong. patch attached.
msg122822 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-29 16:00
Thanks for the patch.

First, you don't need to support str, since sockets only accept binary strings (not unicode).

Second, I think it's simpler and more generic to do something like:

  try:
      self.sock.sendall(data)
  except TypeError:
      try:
          it = iter(data)
      except TypeError:
          raise TypeError("data should be a bytes-like object or "
              "an iterable, got %r" % type(it))
      for d in t:
          self.sock.sendall(d)
msg122878 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-30 03:20
pitrou: that sounds good. I attached another patch.
msg122898 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-30 11:00
pitrou: actually that seems a bit suspect now... you need to handle 'data' differently depending on its type, and while you can determine the type by finding out when 'data' throws certain exceptions, it doesn't seem like what exceptions were meant for.
msg122904 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-30 13:27
> pitrou: actually that seems a bit suspect now... you need to handle
> 'data' differently depending on its type,

Yes, but you can't know all appropriate types in advance, so it's better
to try and catch the TypeError.

I don't understand your changes in Lib/urllib/request.py. len(data) will
raise anyway.
msg122905 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-30 14:53
I don't fully understand Lib/urllib/request.py either, I just ported it and ran the unittests... it seems like what it does is that if you send an iterator through as 'data' you can't know the length in advance, and rather than let the len(data) raise an exception catlee thought it's better to raise an exception to tell the user exactly why the code failed (ie, because the user sent an iterator and there's no way to meaningfully find the Content-Length of that).

As for the catching exceptions vs using isinstance: I thought about it for a while, I think something like this feels right to me:

  try:
      self.sock.sendall(data)
  except TypeError:

      if isinstance(data, collections.Iterable):
          for d in t:
              self.sock.sendall(d)
      else:
          raise TypeError("data should be a bytes-like object or an iterable, got %r" % type(it))


anyway, calling iter(data) is equivalent to calling data.__iter__(), so catching the exception is equivalent to hasattr(data, '__iter__'), which is roughly the same as isinstance(data, collections.Iterable). so we try the most straightforward method (sending everything) then if that fails, data is either an iterator or a wrong type.
msg122906 - (view) Author: Davide Rizzo (davide.rizzo) * Date: 2010-11-30 15:35
> len(data) will raise anyway.

No, it won't, if the iterable happens to be a sequence.
msg122907 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-30 15:43
> > len(data) will raise anyway.
> 
> No, it won't, if the iterable happens to be a sequence.

Well, it seems the patch is confused between iterable and iterator. Only
iterators have a __next__, but they usually don't have a __len__.

The patch should really check for iterables, so it should use:

    if isinstance(data, collections.Iterable)
        raise ValueError#etc.
msg122909 - (view) Author: Xuanji Li (xuanji) Date: 2010-11-30 15:55
davide: yeah, hasattr("lol", '__next__') == False, even though strings are Iterable; so for strings and other such sequences the len(data) line will be executed. So technically we shouldn't say "No Content-Length specified for iterable body" but we should say "No Content-Length specified for iterable body that is not a sequence". 

Basically, this whole patch (both parts of it) will be much better off iif there is a clean way to say "a is an iterable but a is not a sequence", because even though b'this is a message' is Iterable, we want to treat it differently compared to, say, a generator object; we do NOT want to use the Iterator features (iter, next) of it, we want to use the sequencey features (by sending the whole chunk of it, by calling len)
msg122910 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-11-30 16:03
Xuanji, a wording which does convey the approximate meaning is fine. I think, the Exception error messages will help the people based on the Context.

- Lets have the ValueError raised from the urllib/request.py. Changing it to isinstance(data,collections.Iterable) as Antoine suggested is okay here too.
- Same change for http.client code checking for an Iterable as specified in msg122905

tests are fine. Docs and NEWS should be updated.

Georg: Is it okay, if we push this feature in before Dec 4th, beta1 release?
msg122911 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-30 16:07
One way to check that it's bytes-compatible is to take a memoryview of it:

>>> memoryview(b"abc")
<memory at 0x1cf5120>
>>> memoryview(bytearray(b"abc"))
<memory at 0x1cf55a0>
>>> memoryview(array.array('b', b"abc"))
<memory at 0x1cf52a0>

>>> memoryview([b"abc"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot make memory view because object does not have the buffer interface
>>> memoryview("abc")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot make memory view because object does not have the buffer interface
msg122912 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-30 16:10
> - Lets have the ValueError raised from the urllib/request.py. Changing
> it to isinstance(data,collections.Iterable) as Antoine suggested is
> okay here too.

Xuanji is right: it's not. We want bytes to be accepted, and it's an
iterable.
msg122915 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-30 16:37
Answering to myself, sorry. memoryview() does return the right answer of whether the object supports the buffer interface, *however* it doesn't mean the len() will be right. For example, take an array.array of ints:

>>> memoryview(array.array("I", [1,2,3]))
<memory at 0x1cf5720>
>>> len(array.array("I", [1,2,3]))
3
>>> len(memoryview(array.array("I", [1,2,3])))
3
>>> len(bytes(array.array("I", [1,2,3])))
12

len() returns 3 but the number of bytes written out by sendall() will really be 12...

*However*, the right len can be calculated using the memoryview:

>>> m = memoryview(array.array("I", [1,2,3]))
>>> len(m) * m.itemsize
12

(without actually converting to a bytes object, so all this is cheap even for very large buffers)
msg122921 - (view) Author: Éric Araujo (merwok) * (Python committer) Date: 2010-11-30 17:23
> hasattr("lol", '__next__') == False, even though strings are Iterable

FYI, magic methods are looked up on the class, not on the instance.  That’s why ABCs are the right thing to use here.  http://docs.python.org/dev/reference/datamodel#special-method-names
msg122970 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-01 01:59
So, reading all your comments, I gather that my proposed patch for client.py which is

  try:
      self.sock.sendall(data)
  except TypeError:
      if isinstance(data, collections.Iterable):
          for d in t:
              self.sock.sendall(d)
      else:
          raise TypeError("data should be a bytes-like object or an iterable, got %r" % type(it))

is ok, because it avoids using hasattr to test for Iterable and avoids isinstance() to check for specific types (str, bytes...) but instead uses exceptions (as pitrou suggested)?

if that is ok with everyone, I just need to work more on request.py to remove the hasattr; I'll probably use the memoryview solution suggested by pitrou.

just to confirm: we WANT array.array("I", [1,2,3]) to have a content-length of 12, right?
msg122973 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-01 03:09
> just to confirm: we WANT array.array("I", [1,2,3]) to have a content-
> length of 12, right?

Yes, since it will emit 12 bytes in the body (you could actually have a test for it).
msg122986 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-01 09:41
Give a try to this minor variation of the patch with tests added and let me know your review comments.
msg122987 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-01 10:06
Senthil:

+        try:
+           self.sock.sendall(data)

Indentation problem here.

+                if isinstance(data,str):
+                    content_length = len(data)

I'm not sure I understand. What does sending an unicode string mean?

+        # Check iterable body support
+        def iterable_body():
+            yield "one"
+            yield "two"
+            yield "three"

Iterables of strings? this doesn't seem supported in the patch.

Also, it would be nice if the tests checked that the sent data is as expected.
msg122996 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-01 14:56
orsenthil: Hi, i don't quite understand why iter() needs to be called explicitly on data? As I understand it, if data is an iterable then you can use a for loop on it directly.
msg122998 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-01 15:49
attaching new patch. this implements the memoryview solution suggested by pitrou. but it does contain this thing:

if not request.has_header('Content-length'):
    if (not hasattr(data, '__read__') and 
        isinstance(data, collections.Iterable)):
        print(data,"is an iterable")
        try:
            m = memoryview(data)
            print(m.itemsize * len(m))
            request.add_unredirected_header(
                'Content-length', '%d' % (len(m) * m.itemsize))
        except TypeError:
            try:
                request.add_unredirected_header(
                    'Content-length', '%d' % len(data))
            except TypeError:
                raise ValueError(
                    "No Content-Length specified for iterable body")

why is it so nested? because data can support 3 different interfaces:

1) Buffer interface, in that case use memoryview to count bytes
2) Can call len but not buffer: assume len == #bytes
3) Iterable but cannot call len or memoryview: raise ValueError

I hope there is a simpler way...
msg122999 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-01 15:50
attaching new patch. this implements the memoryview solution suggested by pitrou. but it does contain this thing:

if not request.has_header('Content-length'):
    if (not hasattr(data, '__read__') and 
        isinstance(data, collections.Iterable)):
        print(data,"is an iterable")
        try:
            m = memoryview(data)
            print(m.itemsize * len(m))
            request.add_unredirected_header(
                'Content-length', '%d' % (len(m) * m.itemsize))
        except TypeError:
            try:
                request.add_unredirected_header(
                    'Content-length', '%d' % len(data))
            except TypeError:
                raise ValueError(
                    "No Content-Length specified for iterable body")

why is it so nested? because data can support 3 different interfaces:

1) Buffer interface, in that case use memoryview to count bytes
2) Can call len but not buffer: assume len == #bytes
3) Iterable but cannot call len or memoryview: raise ValueError

I hope there is a simpler way...
msg123001 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-01 17:08
> if not request.has_header('Content-length'):
>     if (not hasattr(data, '__read__') and 

What is __read__ supposed to be?

> 2) Can call len but not buffer: assume len == #bytes

Why do you need it at all?
msg123038 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 02:19
On Wed, Dec 01, 2010 at 10:06:25AM +0000, Antoine Pitrou wrote:
> +        try:
> +           self.sock.sendall(data)
> 
> Indentation problem here.

I could notice it now. Shall fix it.

> 
> +                if isinstance(data,str):
> +                    content_length = len(data)
> 
> I'm not sure I understand. What does sending an unicode string mean?

That's my mistake with understanding, I just realized (again) that
socket.send, socket.sendall does only bytes. And we don't encode the
unicode code string to send as bytes too.

> +        def iterable_body():
> +            yield "one"
> +            yield "two"
> +            yield "three"
> 
> Iterables of strings? this doesn't seem supported in the patch.

It should be:

+            yield b"one"
+            yield b"two"
+            yield b"three"
msg123039 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 02:23
On Wed, Dec 01, 2010 at 02:56:56PM +0000, Xuanji Li wrote:
> orsenthil: Hi, i don't quite understand why iter() needs to be
> called explicitly on data? As I understand it, if data is an
> iterable then you can use a for loop on it directly.
> 

The reasoning I followed was, data is an "Iterable" (a collection) and
you get an "Iterator" by passing via iter(). And you send the items by
looping over the iterator.

Honestly, I am not sure if iter is needed here too. I thought it was
not needed too, when you determine it is an Iterable and iterate over
it using the for loop. But I kept the iter() method just to create an
instance and send it.

Antoine, which would be the correct/ better?
msg123040 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 02:40
On Wed, Dec 01, 2010 at 05:08:26PM +0000, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> > if not request.has_header('Content-length'):
> >     if (not hasattr(data, '__read__') and 
> 
> What is __read__ supposed to be?

I don't think is required. The previous 2.x version patch was doing
this just to ensure that it is not file object and then it is a
sequence. (I could not understand why)

Now, when you determine that the sequence can be bytes, bytearray or
array.array then testing for memory view is enough. File objects
without Content-Length would raise an Exception too.

Not required. For the same reason as above.
msg123043 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 02:51
On Thu, Dec 02, 2010 at 02:19:10AM +0000, Senthil Kumaran wrote:
> On Wed, Dec 01, 2010 at 10:06:25AM +0000, Antoine Pitrou wrote:
> > +        try:
> > +           self.sock.sendall(data)
> > 
> > Indentation problem here.
> 
> I could notice it now. Shall fix it.

Sorry, there was not any, if you viewed the patch online, it looked
as if there was, but not really. (I was surprised as how it could be
and not be caught by tests in the first place).
msg123044 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-02 03:08
> > 2) Can call len but not buffer: assume len == #bytes

> Why do you need it at all?

Hmm, I'm looking at the the tests in urllib2 that fail if we omit this... in test_urllib2 there are tests that do this:

req = Request("http://example.com/", "")

and they expect Content-Length to be set to 0...
msg123045 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 03:14
Updated patch after correcting the mistake (bytes vs str) in the previous one.
msg123046 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 03:15
On Thu, Dec 02, 2010 at 03:08:55AM +0000, Xuanji Li wrote:
> 
> req = Request("http://example.com/", "")

That should be:

> req = Request("http://example.com/", b"")

I updated some of those in the latest updated patch.
msg123047 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-02 03:16
And my version too...
msg123049 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-02 03:19
Actually I don't think you can go around changing test_urllib2.py, they are after all regression tests... and surely some users will send "" as data.

My patch handles the case of 0-length data specially, removing the need for 2) Can call len but not buffer: assume len == #bytes
msg123050 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 03:22
On Thu, Dec 02, 2010 at 03:16:53AM +0000, Xuanji Li wrote:
> And my version too...
> 

+                if hasattr(data, '__len__') and not len(data):
+                    request.add_unredirected_header('Content-length', '0')

This is very special case. It should not be so. There was wrong
examples in the test_urllib2 which I just corrected.

Expect for the difference in (it = iter(data) - Which I am seeking
some advice too). Rest of the things in both patches are same.

Thank you.
msg123051 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 03:23
On Thu, Dec 02, 2010 at 03:20:05AM +0000, Xuanji Li wrote:
> Actually I don't think you can go around changing test_urllib2.py,
> they are after all regression tests... and surely some users will
> send "" as data.
> 

Think about it this way. In py3k, socket.send can handle only bytes,
not string. So sending "" as data to the socket.send is wrong.
msg123052 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-02 03:28
thought of a better way, we can check if data is true; this will cover "" and b"". this is in issue_3243_py3k_7.patch
msg123053 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-02 03:45
Glaring at my mistakes.

> There was wrong examples in the test_urllib2 which I just corrected.

There were..

> Expect for the difference in (it = iter(data) - Which I am seeking

Except for ...
msg123069 - (view) Author: Éric Araujo (merwok) * (Python committer) Date: 2010-12-02 12:47
>> What is __read__ supposed to be?
> I don't think is required.

The point is that Python does not define a __read__ magic method.  Only read exists, on file objects.
msg123073 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-02 14:12
eric: sorry, that has been fixed in issue_3243_py3k_7.patch
msg124339 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-19 10:52
This is committed in r87399. Documentation and NEWS is added. Thanks for the patch and review comments.
msg124340 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-19 12:24
Hello, sorry for commenting on a closed issue... but I think the documentation change is incorrect. In urllib.request.rst, it says data is a string. However as seen in the changes to test_urllib2.py, data must be a bytes object rather than a string object. I think we should reopen this issue and change the documentation.

Thanks!
msg124341 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-19 12:28
Also, the patch for request.py contains a debug statement, print(data)
msg124342 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-19 12:34
Good catch, fixed in r87400.
msg124343 - (view) Author: Xuanji Li (xuanji) Date: 2010-12-19 12:47
Also, I am not familiar with the backward-comparability requirements of py3k, but orsenthil's patch will break py3k code that relies on data being a string, shouldn't this be mentioned somewhere?

Finally, I do not understand why my proposed change, which is to add

+    if not data:
+        request.add_unredirected_header('Content-length', '0')


so that code that relies on being able to pass a blank string as data (for instance, the code in test_urllib2.py) is still able to do so.
msg124344 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-19 12:58
Hmm, indeed: Senthil, could data be a string in earlier versions?

If yes, the code should be changed to still allow that.  (But after beta2 please, it's already tagged.)
msg124345 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-19 13:03
Raising priority so that this gets sorted out before final.
msg124355 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-19 14:46
Xuanji, Thanks for the comments on 'data' being bytes. I had just cared to add the feature information. I think that data detail should have been updated too. 

I think for your other two questions, we discussed it msg123051 - socket in py3k handles only bytes, sending string was wrong and test_urllib2 had the mistake in sending zero length strings which weren't detected.

However, let me see if we have to accommodate those "very special case" where data can be a zero length string just to accommodate the mistake it was present in the earlier version.
msg124588 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-12-24 04:05
A mistake with Content-Length in the previous commit resolved in revision 87469.
msg126326 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-15 13:12
Anything left to do here, Senthil?
msg126342 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-01-15 17:32
Hi Georg,

In the previous comment, I had written that

'let me see if we have to accommodate those "very special case"
where data can be a zero length string just to accommodate the mistake
it was present in the earlier version.'

More, I think about it, the more it seems to me that accommodating that
special wrong case is not required.

The decision boils down to this.

1. In py3k, data for POST should be bytes.

2. But urllib.request had a 'bug/hole' that when a zero length string
was passed as a data, it did not raise an exception. There were cases
in test_urllib2 where zero length string was passed. I argue that it
was more of a mistake than, what we actually wanted to test it.

Because there is NO practical scenario where Zero length data as a
POST is useful.

3. Now, with the introduction of this feature requested in this issue,
this zero length string would raise an Exception and would demand that
even if it is zero length, please send it as bytes.

IMO, this is correct behavior and we need not accommodate the previous
one. 

So, I would recommend closing this bug as Fixed without further change.
At most, a NEWS item can be added to explain point 2.
msg126344 - (view) Author: Xuanji Li (xuanji) Date: 2011-01-15 17:41
Yes, I think we should close it.
msg126346 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2011-01-15 18:26
Thanks for the note. I shall some details to the NEWS entry before the release. Closing this report.
History
Date User Action Args
2011-01-15 18:27:50orsenthilsetpriority: low -> normal
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
2011-01-15 18:26:50orsenthilsetstatus: open -> closed
priority: critical -> low

messages: + msg126346
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
2011-01-15 17:41:11xuanjisetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg126344
2011-01-15 17:32:41orsenthilsetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg126342
2011-01-15 16:39:51georg.brandlsetpriority: deferred blocker -> critical
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
2011-01-15 13:12:13georg.brandlsetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg126326
2010-12-24 04:05:30orsenthilsetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124588
2010-12-19 14:46:42orsenthilsetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124355
2010-12-19 13:03:17georg.brandlsetpriority: normal -> deferred blocker
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124345
2010-12-19 12:58:50georg.brandlsetstatus: closed -> open
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124344
2010-12-19 12:47:10xuanjisetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124343
2010-12-19 12:34:03georg.brandlsetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124342
2010-12-19 12:28:50xuanjisetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124341
2010-12-19 12:24:31xuanjisetnosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124340
2010-12-19 10:52:35orsenthilsetstatus: open -> closed
nosy: jhylton, georg.brandl, rhettinger, orsenthil, pitrou, catlee, merwok, rcoyner, xuanji, davide.rizzo
messages: + msg124339

resolution: fixed
stage: needs patch -> resolved
2010-12-02 14:12:19xuanjisetmessages: + msg123073
2010-12-02 12:47:00merwoksetmessages: + msg123069
2010-12-02 03:45:32orsenthilsetmessages: + msg123053
2010-12-02 03:28:46xuanjisetfiles: + issue_3243_py3k_7.patch

messages: + msg123052
2010-12-02 03:23:45orsenthilsetmessages: + msg123051
2010-12-02 03:22:28orsenthilsetmessages: + msg123050
2010-12-02 03:19:49xuanjisetmessages: + msg123049
2010-12-02 03:16:50xuanjisetfiles: + issue_3243_py3k_6.patch

messages: + msg123047
2010-12-02 03:15:47orsenthilsetmessages: + msg123046
2010-12-02 03:14:10orsenthilsetfiles: + Issue3243-6.patch

messages: + msg123045
2010-12-02 03:08:54xuanjisetmessages: + msg123044
2010-12-02 02:51:30orsenthilsetmessages: + msg123043
2010-12-02 02:40:50orsenthilsetmessages: + msg123040
2010-12-02 02:23:19orsenthilsetmessages: + msg123039
2010-12-02 02:19:08orsenthilsetmessages: + msg123038
2010-12-01 17:08:24pitrousetmessages: + msg123001
2010-12-01 15:51:20xuanjisetfiles: - issue_3243_py3k_5.patch
2010-12-01 15:50:26xuanjisetfiles: + issue_3243_py3k_5.patch

messages: + msg122999
2010-12-01 15:49:51xuanjisetfiles: + issue_3243_py3k_5.patch

messages: + msg122998
2010-12-01 14:56:54xuanjisetmessages: + msg122996
2010-12-01 10:06:24pitrousetmessages: + msg122987
2010-12-01 09:41:57orsenthilsetfiles: + Issue3243-4.patch

messages: + msg122986
2010-12-01 03:09:53pitrousetmessages: + msg122973
2010-12-01 01:59:53xuanjisetmessages: + msg122970
2010-11-30 17:23:50merwoksetmessages: + msg122921
2010-11-30 16:37:46pitrousetmessages: + msg122915
2010-11-30 16:10:26pitrousetmessages: + msg122912
2010-11-30 16:07:31pitrousetmessages: + msg122911
2010-11-30 16:03:05orsenthilsetnosy: + georg.brandl
messages: + msg122910
2010-11-30 15:55:42xuanjisetmessages: + msg122909
2010-11-30 15:43:49pitrousetmessages: + msg122907
2010-11-30 15:35:34davide.rizzosetnosy: + davide.rizzo
messages: + msg122906
2010-11-30 14:53:44xuanjisetmessages: + msg122905
2010-11-30 13:27:48pitrousetmessages: + msg122904
2010-11-30 11:00:02xuanjisetmessages: + msg122898
2010-11-30 03:20:14xuanjisetfiles: + issue_3243_py3k_3.patch

messages: + msg122878
2010-11-29 16:00:04pitrousetnosy: + pitrou
messages: + msg122822
2010-11-29 15:53:46xuanjisetfiles: + issue_3243_py3k_2.patch

messages: + msg122819
2010-11-29 11:23:42xuanjisetmessages: + msg122787
2010-11-29 09:15:06orsenthilsetmessages: + msg122784
2010-11-29 01:46:28xuanjisetmessages: + msg122756
2010-11-28 19:10:13rhettingersetnosy: + rhettinger
messages: + msg122707
2010-11-28 10:39:08xuanjisetfiles: + issue_3243_py3k.diff
nosy: + xuanji
messages: + msg122647

2010-11-28 02:56:32orsenthilsetassignee: jhylton -> orsenthil
2010-11-28 02:34:19merwoksetkeywords: + easy
resolution: accepted -> (no value)
stage: patch review -> needs patch
2010-11-12 00:46:30merwoksetnosy: - BreamoreBoy
messages: + msg120999
2010-07-18 19:32:09BreamoreBoysetnosy: + BreamoreBoy
messages: + msg110675
2010-06-26 08:27:03merwoksetkeywords: + needs review
nosy: + orsenthil
stage: test needed -> patch review

versions: - Python 2.7
2010-06-04 11:53:40merwoksetnosy: + merwok
2010-03-11 00:02:12rcoynersetnosy: + rcoyner
messages: + msg100817
2009-05-16 20:34:20ajaksu2setpriority: normal
stage: test needed
versions: + Python 3.2
2009-03-28 12:21:08jhyltonsetnosy: + jhylton
messages: + msg84304

assignee: jhylton
resolution: accepted
2009-02-12 17:47:25ajaksu2linkissue3244 dependencies
2008-12-05 16:43:44catleesetfiles: + python-3243.patch
keywords: + patch
messages: + msg77037
2008-06-30 18:01:18catleecreate