Senthil Kumaran added the comment:

I spent time digging for a proper fix for this issue.

I've come to a conclusion that this commit, 
https://hg.python.org/cpython/rev/44d02a5d59fb (10 May 2016) in 2.7.12, was a 
mistake and needs to be reverted.

The reason this change was made was apparently, it fixed a particular problem 
with retrieving files in 3.x, and the change was backported (issue26960). It 
was reported and fixed in 3.x code line here http://bugs.python.org/issue16270 
(This is an improper change too and it needs to be reverted, we will come to it 
at the end [3].

Why is this a problem?

1. The change made was essentially taking the logic of draining ftp response 
from endtransfer method. Historically, in the module, endtransfer() has been 
used before closing the connection and it is correct to drain response 
(voidresp() method call).

2. If we don't drain responses, we end up with problems like the current 
(issue27973) bug report.

3. The problem with issue16270 was the fail transfer failed only when urllopen 
was used as a context manager (which calls close implicitly on the same host). 
But ftp scenarios were designed for reusing the same host without calling close 
from a long time (3a) and context manager scenario is not applicable to 2.7 
code (3b).

Here are some references on how the module shaped up:
 
3a. https://hg.python.org/cpython/rev/6e0eddfa404a - it talks about repeated 
retriving of the same file from host without closing as a feature.

3b. The urllopen context manager was applicable only in 3.x so the original 
problem of issue16270 was not reproducible in 2.7 and it was improper to 
backport those changes (issue26960). Even issue16270 it is improper to change 
the code in endtransfer, because the problem is specific with context-manager 
usecase of urlopen, regular usage is just fine.

This patch fixes the problem for 2.7. I have included tests in test_urllibnet 
to cover the scenarios that were reported. Please review this.

For 3.x code, I will reopen issue16270, I will port this patch with test cases 
and an additional case for context manager scenario.

----------
Added file: http://bugs.python.org/file46381/issue27973.patch

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue27973>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to