Re: [Python-Dev] cpython (3.2): Fix closes Issue12576 - fix urlopen behavior on sites which do not send (or
On Wed, 27 Jul 2011 02:25:56 +0200 senthil.kumaran <python-checkins@python.org> wrote:
+ def test_sites_no_connection_close(self): + # Some sites do not send Connection: close header. + # Verify that those work properly. (#issue12576) + + try: + with urllib.request.urlopen('http://www.imdb.com') as res: + pass
Can you please at least use support.transient_internet() as in other tests in this file?
+ except ValueError as e: + self.fail("urlopen failed for sites not sending Connection:close") + else: + self.assertTrue(res) + + req = urllib.request.urlopen('http://www.imdb.com')
Why a second time?
+ res = req.read() + self.assertTrue(res)
Also, when does "req" get closed? Right now I get resource warnings: test_sites_no_connection_close (test.test_urllib2net.OtherNetworkTests) ... /home/antoine/cpython/default/Lib/socket.py:342: ResourceWarning: unclosed <socket.socket object, fd=3, family=2, type=1, proto=6> self._sock = None /home/antoine/cpython/default/Lib/socket.py:342: ResourceWarning: unclosed <socket.socket object, fd=3, family=2, type=1, proto=6> self._sock = None ok Regards Antoine.
On Wed, Jul 27, 2011 at 11:52:32AM +0200, Antoine Pitrou wrote:
+ + try: + with urllib.request.urlopen('http://www.imdb.com') as res: + pass
Can you please at least use support.transient_internet() as in other tests in this file?
It was intentional because ValueError was raised from context manager use case for a bug where the request object was closed prematurely. support.transient_internet, I believe would not have covered that case (Usage scenario).
+ req = urllib.request.urlopen('http://www.imdb.com')
Why a second time?
When used outside of context manager, it gave empty response instead of ValueError and the test case was to check that.
+ res = req.read() + self.assertTrue(res)
Also, when does "req" get closed? Right now I get resource warnings:
I shall fix this one. I think, attempting to fix the Resource warning caused the regression wherein the request object closed prematurely. I shall look at this. Thanks, Senthil
On Wed, 27 Jul 2011 20:16:01 +0800 Senthil Kumaran <senthil@uthcode.com> wrote:
On Wed, Jul 27, 2011 at 11:52:32AM +0200, Antoine Pitrou wrote:
+ + try: + with urllib.request.urlopen('http://www.imdb.com') as res: + pass
Can you please at least use support.transient_internet() as in other tests in this file?
It was intentional because ValueError was raised from context manager use case for a bug where the request object was closed prematurely. support.transient_internet, I believe would not have covered that case (Usage scenario).
Unless I'm reading wrongly, transient_internet doesn't silence ValueError at all.
+ res = req.read() + self.assertTrue(res)
Also, when does "req" get closed? Right now I get resource warnings:
I shall fix this one. I think, attempting to fix the Resource warning caused the regression wherein the request object closed prematurely. I shall look at this.
Well, the test should simply call close() as is done in other tests. Regards Antoine.
On Wed, Jul 27, 2011 at 02:21:59PM +0200, Antoine Pitrou wrote:
transient_internet doesn't silence ValueError at all.
Yes, that is correct. I missed recollecting it in the first place. I guess, I did not see using a content manager withing another context manager block as something nice.. (nothing wrong but just the indented code with additional wrap of exceptional handler). But yes, it would be better to wrap it with transient_internet call. Shall do.
Well, the test should simply call close() as is done in other tests.
I tried that before responding, it does not silence the Resource Warning. The fix (at least for these cases where Connection:close header is not sent) lies in closing request in the urllib.request. It is tricky as the server is not sending a Connection:close header which httplib relies on to close the socket. -- Senthil
participants (2)
-
Antoine Pitrou -
Senthil Kumaran