[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

The backported package now exists on PyPI as zipfile36. It's passing the tests 
on 3.4 - I haven't tested further back. I'm trying out Gitlab for hosting it:
https://gitlab.com/takluyver/zipfile36

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

LGTM. Thank you Thomas.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Roundup Robot]

Roundup Robot added the comment:

New changeset 29b470b8ab77 by Serhiy Storchaka in branch 'default':
Issue #26039: Document ZipInfo.is_dir() and make force_zip64 keyword-only.
https://hg.python.org/cpython/rev/29b470b8ab77

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Updated version of the patch with the detail moved to rst.

--
Added file: http://bugs.python.org/file42855/zipfile-flex-bonus2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Agreed that the docstring should be shorten version of the documentation and 
not vice versa. The directory slash information is implementation detail. I'm 
not sure it is needed in the documentation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Martin Panter]

Martin Panter added the comment:

The bonus patch looks okay, although I wonder if the directory slash (/) 
information should be in the RST rather than doc string. Usually the RST has 
all the details, and doc strings are just summaries.

Regarding exceptions, I can sympathise with both sides of the argument and 
don’t have a strong opinion (why does the exception type matter for programmer 
errors anyway?). But I think it might be better to be locally consistent within 
the zipfile module, and the module is already heavily documented with 
RuntimeError for similar programmer errors.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Patch attached for points 2 and 3

--
Added file: http://bugs.python.org/file42851/zipfile-flex-bonus.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

1. For the cases like read/write handles conflicting, ValueError doesn't seem 
quite right to me - I take ValueError to mean that you have passed an invalid 
value to a function. RuntimeError feels like the closest fit for 'the current 
state does not allow this operation'. It is rather broad, though.

If I was writing it anew, I would use ValueError for something like an invalid 
mode parameter: zf.open('foo', mode='q') . That particular case was already in 
the code, though, so I think backwards compatibility should take precedence.

2 & 3. Agreed, I'll prepare a patch

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

There are yet few issues.

1. Currently RuntimeError is widely used in zipfile. But in most cases 
ValueError would be more appropriate since it programming error (for example 
ValueError is raised when try to read or write to closed file). See issue24693, 
but this is backward incompatible change. For new exceptions we are free to use 
any exception type without breaking backward compatibility. It looks to me, 
that ValueError is more appropriate is these cases than RuntimeError. What do 
you think about this Thomas? Here is a patch that changes exceptions types.

2. ZipInfo.is_dir() is not documented and lacks a docstring.

3. It would be better to make force_zip64 a keyword-only parameter. This would 
allow to add new positional parameters without breaking compatibility.

--
resolution: fixed -> 
stage: resolved -> patch review
status: closed -> open
Added file: http://bugs.python.org/file42850/zipfile-open-w-exceptions.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Let me know when you create a backport package Thomas. I'd like to help.

--
stage: commit review -> resolved

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Thanks Serhiy! I am marking this as fixed.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Roundup Robot]

Roundup Robot added the comment:

New changeset 175c3f1d0256 by Serhiy Storchaka in branch 'default':
Issue #26039: zipfile.ZipFile.open() can now be used to write data into a ZIP
https://hg.python.org/cpython/rev/175c3f1d0256

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Martin Panter]

Martin Panter added the comment:

Yes this looks okay to me, so go ahead.

My only concern was other methods affecting the file position while in the 
middle of writing, but it looks like those methods maintain separate objects 
like self.filelist, so there is no conflict.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your contribution Thomas. I'll push the patch with fixed writestr 
in short time unless Martin left comments.

--
stage: patch review -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Thanks Serhiy. I'm quite happy with that set of constraints. I had a look over 
your patch set, and just spotted one thing in writestr that I have overlooked 
all along.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I have added comments for tests and documentation. The implementation looks 
correct. But I prefer simpler solution on this stage. Here is something between 
zipfile-open-w5.patch and zipfile-open-w8.patch, close to zipfile-open-w5.patch 
with fixed issues.

* Writer doesn't use look and seeking.
* Writing while there are open read handlers is permitted.
* Reading while there is open write handler is forbidden.

This works in all cases when current code works. The ability to read while 
there is open write handler is separate new feature (and I don't know whether 
there is a need in it).

--
Added file: http://bugs.python.org/file42819/zipfile-open-w9.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Ping? Once this is landed, I intend to make a backport package of it so I can 
start using it before Python 3.6 comes out.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Марк Коренберг]

Changes by Марк Коренберг :


--
nosy:  -mmarkk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Any further review comments on this?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

zipfile-open-w8 removes the concurrent read-write check for non-seekable files. 
As Martin points out, reading currently requires seeking, and a streaming read 
API would look very different from the current API.

--
Added file: http://bugs.python.org/file42598/zipfile-open-w8.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

zipfile-open-w7 adds a test that Martin requested

--
Added file: http://bugs.python.org/file42597/zipfile-open-w7.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Martin Panter]

Martin Panter added the comment:

I understand Python’s zipfile module does not allow reading unless seek() is 
supported (this was one of Марк’s complaints). So it is irrelevant whether we 
are writing.

I left a few comments, but it sounds like it is valid read and write at the 
same time.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

It wasn't as bad as I thought (hopefully that doesn't mean I've missed 
something). zipfile-open-w6.patch allows interleaved reads and writes so long 
as the underlying file object is seekable.

You still can't have multiple write handles open (it wouldn't know where to 
start the second file), or read & write handles on a non-seekable stream (I 
don't think a non-seekable read-write file ever makes sense, except perhaps for 
a pty, which is really two separate streams on one file descriptor).

Tests are passing again.

--
Added file: http://bugs.python.org/file42588/zipfile-open-w6.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

test_write_after_read was added in issue14099. It doesn't test intended 
behavior, it tests existing behavior just in case to not change it 
unintentionally. It is desirable to preserve it, but if there is no simple way, 
may be we can change it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Martin, Serhiy: Am I right that it would be unacceptable to change the test, 
and I therefore need to find a way to allow writestr while a reading-mode 
handle is open?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Thomas Kluyver]

Thomas Kluyver added the comment:

Sorry for the delay, this fell off my radar because emails from both the bug 
tracker and Rietveld tend to fall foul of my spam filters, so I have to go and 
check.

I have implemented Serhiy's suggestions, but there turns out to be a test 
(test_write_after_read) that calls writestr while a reading handle is open, and 
it now fails. This is absolutely expected according to the design we discussed, 
but if there's a test, I guess that means we need to keep that functionality 
working?

In principle, the only thing that's not possible is interleaving writes to 
multiple files within the zip - because we don't know where to start writing 
the second file. We should be able to have one writer and n readers going at 
once, but every time I start looking into that I get mired in complexity. Maybe 
(hopefully) there's some critical abstraction that hasn't occurred to me.

--
Added file: http://bugs.python.org/file42375/zipfile-open-w5.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Марк Коренберг]

Марк Коренберг added the comment:

https://github.com/SpiderOak/ZipStream tries to implement streaming in one more 
kind.

Also libarchive have experimental support of streaming write to zip archives in 
newest version.

So problem is actual.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26039] More flexibility in zipfile write interface

[Martin Panter]

Martin Panter added the comment:

Mark: I suggest to keep this discussion focussed on writing zip files, although 
I agree some enhancements to help reading without seeking could be reasonable.

--
title: More flexibility in zipfile interface -> More flexibility in zipfile 
write interface

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com