Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(309)

#21463: RuntimeError when URLopener.ftpcache is full

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 8 months ago by erik.m.bray
Modified:
5 years, 7 months ago
Reviewers:
benjamin, nevermaker
CC:
orsenthil, Benjamin Peterson, eric.araujo, jesstess, devnull_psf.upfronthosting.co.za, erik.bray, shiinee
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 3

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_urllib.py View 1 2 4 chunks +31 lines, -1 line 0 comments Download
Lib/urllib/request.py View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 2
Benjamin Peterson
Thank you for the patch. It looks good; I only have a few suggested improvements. ...
5 years, 7 months ago #1
shiinee
5 years, 7 months ago #2
On 2014/06/07 22:37:26, Benjamin Peterson wrote:
> Thank you for the patch. It looks good; I only have a few suggested
> improvements.
> 
> You can also run "make patchcheck" to remove trailing whitespace from your
> patch.
> 
> https://bugs.python.org/review/21463/diff/12081/Lib/test/test_urllib.py
> File Lib/test/test_urllib.py (right):
> 
>
https://bugs.python.org/review/21463/diff/12081/Lib/test/test_urllib.py#newco...
> Lib/test/test_urllib.py:100: return io.StringIO(), 0
> It doesn't actually matter in this batch, but this should probably be
> io.BytesIO() rather than io.StringIO(), since the real retrfile returns a
binary
> file.
fixed
> 
>
https://bugs.python.org/review/21463/diff/12081/Lib/test/test_urllib.py#newco...
> Lib/test/test_urllib.py:333: urllib.request.MAXFTPCACHE = 0
> This will gobally set the ftp cache to be empty after this test runs! I
suggest
> using the atch() decorator from the unittest.mock module to ensure MAXFTPCACHE
> is reset to its old value after the test runs.
This is new to me, but I think I have it right - please tell me if it looks
good!
> 
>
https://bugs.python.org/review/21463/diff/12081/Lib/test/test_urllib.py#newco...
> Lib/test/test_urllib.py:334: self.fakeftp()
> I suggest wrapping the rest of the the function in a try/finally like the
other
> tests using fakehttp or doing self.addCleanup(self.unfakeftp) after this line.
> This will ensure FTP is "unfaked" whether the test fails or not.
good point, fixed!
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+