[question] It seems to me that you can get streaming by just observing the chunks being written into the file object, and having a generator as well is redundant. I see this is basically needed because PEP333 strongly deprecates having streaming data sent to a write function returned by a generator.
Perhaps it would at least be good to explain that it is done this way for the sake of PEP333.
[question] Given that wsgi does actually support having write() called repeatedly for streaming data, why not use it? I realize it's not the recommended interface, but it is the interface that does fit most naturally with the lower level tarball code we're using. It seems a bit like we're reimplementing something that could be better done in the wsgi framework -- specifically, turning push commands into pull. That would have the concrete benefit of allowing streaming just exactly as data is produced by the compressor, on finer granularity than exporting a single file.
+
+def get_export_generator(tree, dest=None, format=None, root=None, subdir=None, filtered=False,
+ per_file_timestamps=False, fileobj=None):
+
+ """Returns a generator that exports the given Tree to the specific destination.
...
+ :param fileobj: Optional file object to use
Factoring out this function makes sense.
[fix] Please make the docstring explain what the generator actually produces. I am confused whether the content is written to the file object or returned in the generator; also how fileobj interacts with dest.
[idea] Actually, perhaps it would be better to do away with 'dest' entirely at this layer? I think it is only there for the sake of heuristics about choosing the format, and then for opening the file, and
[fix] I see all the actual 'exporters', and also 'export' itself now return generators which must be consumed for them to work. Generally if we change an api in such a way that existing callers will be wrong (rather than getting an error), we ought to rename the api too, and of course add a news entry. Another option would be to keep the existing function names doing the whole job, and add new ones that are generators.
I commend this patch ;-)
[question] It seems to me that you can get streaming by just observing the chunks being written into the file object, and having a generator as well is redundant. I see this is basically needed because PEP333 strongly deprecates having streaming data sent to a write function returned by a generator.
Perhaps it would at least be good to explain that it is done this way for the sake of PEP333.
[question] Given that wsgi does actually support having write() called repeatedly for streaming data, why not use it? I realize it's not the recommended interface, but it is the interface that does fit most naturally with the lower level tarball code we're using. It seems a bit like we're reimplementing something that could be better done in the wsgi framework -- specifically, turning push commands into pull. That would have the concrete benefit of allowing streaming just exactly as data is produced by the compressor, on finer granularity than exporting a single file.
+ generator( tree, dest=None, format=None, root=None, subdir=None, filtered=False, timestamps= False, fileobj=None):
+def get_export_
+ per_file_
+
+ """Returns a generator that exports the given Tree to the specific destination.
...
+ :param fileobj: Optional file object to use
Factoring out this function makes sense.
[fix] Please make the docstring explain what the generator actually produces. I am confused whether the content is written to the file object or returned in the generator; also how fileobj interacts with dest.
[idea] Actually, perhaps it would be better to do away with 'dest' entirely at this layer? I think it is only there for the sake of heuristics about choosing the format, and then for opening the file, and
[fix] I see all the actual 'exporters', and also 'export' itself now return generators which must be consumed for them to work. Generally if we change an api in such a way that existing callers will be wrong (rather than getting an error), we ought to rename the api too, and of course add a news entry. Another option would be to keep the existing function names doing the whole job, and add new ones that are generators.
+def export_ tarball_ item(tree, ball, root, dp, ie, subdir=None, filtered=False,
+ force_mtime=None):
Lifting this out is good.
[tweak] Per pep8, we indent continuation lines either by 4 spaces, or to be just after the preceding parenthesis.