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

#26039: More flexibility in zipfile interface

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 9 months ago by thomas
Modified:
1 year, 5 months ago
Reviewers:
vadmium+py, storchaka
CC:
devnull_psf.upfronthosting.co.za, Thomas Kluyver, Martin Panter, storchaka, mbussonn, mishra.dhiraj95_gmail.com
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 21

Patch Set 5 #

Total comments: 26

Patch Set 6 #

Total comments: 2

Patch Set 7 #

Patch Set 8 #

Total comments: 14

Patch Set 9 #

Total comments: 2

Patch Set 10 #

Total comments: 40

Patch Set 11 #

Total comments: 2

Patch Set 12 #

Total comments: 3

Patch Set 13 #

Patch Set 14 #

Total comments: 10

Patch Set 15 #

Total comments: 5

Patch Set 16 #

Patch Set 17 #

Patch Set 18 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/zipfile.rst View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +10 lines, -2 lines 0 comments Download
Lib/zipfile.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +3 lines, -1 line 0 comments Download

Messages

Total messages: 21
Martin Panter
https://bugs.python.org/review/26039/diff/16429/Doc/library/zipfile.rst File Doc/library/zipfile.rst (right): https://bugs.python.org/review/26039/diff/16429/Doc/library/zipfile.rst#newcode483 Doc/library/zipfile.rst:483: to make a :class:`ZipInfo` instance for a filesystem file ...
1 year, 8 months ago #1
takowl_gmail.com
Thanks for the review; updated patch coming. http://bugs.python.org/review/26039/diff/16433/Doc/library/zipfile.rst File Doc/library/zipfile.rst (left): http://bugs.python.org/review/26039/diff/16433/Doc/library/zipfile.rst#oldcode211 Doc/library/zipfile.rst:211: .. method:: ...
1 year, 8 months ago #2
takowl_gmail.com
http://bugs.python.org/review/26039/diff/16429/Doc/library/zipfile.rst File Doc/library/zipfile.rst (right): http://bugs.python.org/review/26039/diff/16429/Doc/library/zipfile.rst#newcode483 Doc/library/zipfile.rst:483: to make a :class:`ZipInfo` instance for a filesystem file ...
1 year, 8 months ago #3
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode498 Lib/zipfile.py:498: zinfo.compress_type = ZIP_STORED compress_type is ZIP_STORED by default. http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode500 ...
1 year, 8 months ago #4
takowl_gmail.com
http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode498 Lib/zipfile.py:498: zinfo.compress_type = ZIP_STORED On 2016/01/29 13:34:54, storchaka wrote: > ...
1 year, 8 months ago #5
takowl_gmail.com
http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode509 Lib/zipfile.py:509: def isdir(self): On 2016/01/29 15:10:13, Thomas Kluyver wrote: > ...
1 year, 8 months ago #6
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode500 Lib/zipfile.py:500: zinfo.compress_size = 0 On 2016/01/29 15:10:13, Thomas Kluyver wrote: ...
1 year, 8 months ago #7
takowl_gmail.com
http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16429/Lib/zipfile.py#newcode500 Lib/zipfile.py:500: zinfo.compress_size = 0 On 2016/01/29 17:58:25, storchaka wrote: > ...
1 year, 8 months ago #8
Martin Panter
https://bugs.python.org/review/26039/diff/16433/Lib/zipfile.py File Lib/zipfile.py (right): https://bugs.python.org/review/26039/diff/16433/Lib/zipfile.py#newcode961 Lib/zipfile.py:961: class _ZipWriteFile: On 2016/01/29 12:59:14, Thomas Kluyver wrote: > ...
1 year, 8 months ago #9
takowl_gmail.com
http://bugs.python.org/review/26039/diff/16433/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/16433/Lib/zipfile.py#newcode961 Lib/zipfile.py:961: class _ZipWriteFile: On 2016/01/30 14:28:41, vadmium wrote: > On ...
1 year, 8 months ago #10
storchaka_gmail.com
LGTM. Committed with fixed two minor details. http://bugs.python.org/review/26039/diff/16459/Doc/library/zipfile.rst File Doc/library/zipfile.rst (right): http://bugs.python.org/review/26039/diff/16459/Doc/library/zipfile.rst#newcode477 Doc/library/zipfile.rst:477: Doubled empty ...
1 year, 8 months ago #11
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/16460/Lib/test/test_zipfile.py File Lib/test/test_zipfile.py (right): http://bugs.python.org/review/26039/diff/16460/Lib/test/test_zipfile.py#newcode461 Lib/test/test_zipfile.py:461: Needed test for open(mode='w'). http://bugs.python.org/review/26039/diff/16460/Lib/test/test_zipfile.py#newcode1446 Lib/test/test_zipfile.py:1446: zipf.writestr('baz', 'abcde') Needed ...
1 year, 7 months ago #12
Thomas Kluyver
http://bugs.python.org/review/26039/diff/16460/Lib/test/test_zipfile.py File Lib/test/test_zipfile.py (right): http://bugs.python.org/review/26039/diff/16460/Lib/test/test_zipfile.py#newcode461 Lib/test/test_zipfile.py:461: On 2016/03/03 16:26:39, storchaka wrote: > Needed test for ...
1 year, 6 months ago #13
Martin Panter
https://bugs.python.org/review/26039/diff/16891/Lib/test/test_zipfile.py File Lib/test/test_zipfile.py (right): https://bugs.python.org/review/26039/diff/16891/Lib/test/test_zipfile.py#newcode1465 Lib/test/test_zipfile.py:1465: r1 = zipf.open('foo.txt', mode='r') Might be good to keep ...
1 year, 6 months ago #14
Thomas Kluyver
http://bugs.python.org/review/26039/diff/16891/Lib/test/test_zipfile.py File Lib/test/test_zipfile.py (right): http://bugs.python.org/review/26039/diff/16891/Lib/test/test_zipfile.py#newcode1465 Lib/test/test_zipfile.py:1465: r1 = zipf.open('foo.txt', mode='r') On 2016/04/26 04:26:06, vadmium wrote: ...
1 year, 6 months ago #15
Martin Panter
https://bugs.python.org/review/26039/diff/17055/Lib/zipfile.py File Lib/zipfile.py (right): https://bugs.python.org/review/26039/diff/17055/Lib/zipfile.py#newcode1394 Lib/zipfile.py:1394: raise RuntimeError("Can't read from non-seekable zip file " On ...
1 year, 6 months ago #16
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/17063/Doc/library/zipfile.rst File Doc/library/zipfile.rst (right): http://bugs.python.org/review/26039/diff/17063/Doc/library/zipfile.rst#newcode240 Doc/library/zipfile.rst:240: 2 GiB, pass ``force_zip64=True`` to ensure that it the ...
1 year, 5 months ago #17
Thomas Kluyver
http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py#newcode1660 Lib/zipfile.py:1660: zinfo = ZipInfo(filename=zinfo_or_arcname, I think it would be good ...
1 year, 5 months ago #18
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py#newcode1660 Lib/zipfile.py:1660: zinfo = ZipInfo(filename=zinfo_or_arcname, On 2016/05/12 11:54:48, Thomas Kluyver wrote: ...
1 year, 5 months ago #19
storchaka_gmail.com
http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py#newcode1660 Lib/zipfile.py:1660: zinfo = ZipInfo(filename=zinfo_or_arcname, On 2016/05/12 11:54:48, Thomas Kluyver wrote: ...
1 year, 5 months ago #20
Thomas Kluyver
1 year, 5 months ago #21
http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py
File Lib/zipfile.py (right):

http://bugs.python.org/review/26039/diff/17219/Lib/zipfile.py#newcode1660
Lib/zipfile.py:1660: zinfo = ZipInfo(filename=zinfo_or_arcname,
On 2016/05/13 12:45:29, storchaka wrote:
> On 2016/05/12 12:06:39, storchaka wrote:
> > On 2016/05/12 11:54:48, Thomas Kluyver wrote:
> > > I think it would be good to set zinfo.file_size before we pass it to open.
> > It's
> > > probably unlikely that someone will pass >2GB of data into this, but it is
> > > possible, and it's a good example for people looking at the code.
> > 
> > Agreed. This is what the documentation suggests.
> 
> Actually it is set at line 1682.

Ah, yes. I don't know how I missed that. Thanks!
Sign in to reply to this message.

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