Handling conversion of .tar.gz files to .zip files when requested.

Handling conversion of .tar.gz files to .zip files when requested.

R=tapted@chromium.org

Committed: https://github.com/tapted/pub-dartlang/commit/c5b7279

[tapted, 2014-01-08 04:09:03]

CL description should mention the config changes too (or, better, it can be a
separate patch).

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions.py
File app/handlers/package_versions.py (right):

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:12: import tarfile
nit: sort each block of imports

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:67: filename =
"packages/{}-{}.tar.gz".format(package_id, id)
nit: I think google convention is single quotes for strings in python, more
below

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:83: for f in tar:
I think there are two variables called `f` here - probably shouldn't be
abbreviated anyway. Maybe `tar_fileobj` is a good name for the outer f and
`file_info` is a good name for the inner `f`

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:84: zip.writestr(f.name,
tar.extractfile(f.name).read())
can you just use `f.tobuf()` instead of `tar.extractfile(f.name).read()` ?

[tapted, 2014-01-08 04:11:42]

also -- we'll probably need a new fork to commit this - I think trying to commit
will currently go to upstream dart..

[pajamallama, 2014-01-09 00:11:04]

Fixed those errors. Should I make a github fork of
https://github.com/dart-lang/pub-dartlang and set upstream to there?

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions.py
File app/handlers/package_versions.py (right):

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:12: import tarfile
On 2014/01/08 04:09:03, tapted wrote:
> nit: sort each block of imports

Done.

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:67: filename =
"packages/{}-{}.tar.gz".format(package_id, id)
On 2014/01/08 04:09:03, tapted wrote:
> nit: I think google convention is single quotes for strings in python, more
> below

Done.

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:83: for f in tar:
On 2014/01/08 04:09:03, tapted wrote:
> I think there are two variables called `f` here - probably shouldn't be
> abbreviated anyway. Maybe `tar_fileobj` is a good name for the outer f and
> `file_info` is a good name for the inner `f`

Done.

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:84: zip.writestr(f.name,
tar.extractfile(f.name).read())
On 2014/01/08 04:09:03, tapted wrote:
> can you just use `f.tobuf()` instead of `tar.extractfile(f.name).read()` ?

Doesn't look like it. Going TarInfo.tobuf() and saving that produces 512 bytes
of binary for every file, so I'm guessing that is only saving the meta-data to
the file. The code I can find on google seems to use
tar_file.extractfile(tar_info.name)

[tapted, 2014-01-09 00:40:11]

On 2014/01/09 00:11:04, pajamallama wrote:
> Fixed those errors. Should I make a github fork of
> https://github.com/dart-lang/pub-dartlang and set upstream to there?

Looks like I already have a fork! - added you as a collaborator. Setting origin
to this should do the trick - https://github.com/tapted/pub-dartlang.git

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions.py
File app/handlers/package_versions.py (right):

https://codereview.appspot.com/48730046/diff/1/app/handlers/package_versions....
app/handlers/package_versions.py:84: zip.writestr(f.name,
tar.extractfile(f.name).read())
On 2014/01/09 00:11:05, pajamallama wrote:
> On 2014/01/08 04:09:03, tapted wrote:
> > can you just use `f.tobuf()` instead of `tar.extractfile(f.name).read()` ?
> 
> Doesn't look like it. Going TarInfo.tobuf() and saving that produces 512 bytes
> of binary for every file, so I'm guessing that is only saving the meta-data to
> the file. The code I can find on google seems to use
> tar_file.extractfile(tar_info.name) 

Ah, lame - looks like tobuf is the header. But! it would be good to avoid the
member lookup twice. I think you can also pass `file_info` to extractfile,
rather than `file_info.name`.

[tapted, 2014-01-09 00:40:42]

and lgtm with the `file_info.name` -> `file_info` fix

[pajamallama, 2014-01-09 00:46:53]

Committed patchset #4 manually as r845fae8 (presubmit successful).

[pajamallama, 2014-01-09 01:54:25]

Committed patchset #4 manually as r600ae73 (presubmit successful).

[pajamallama, 2014-01-09 02:40:35]

Committed patchset #6 manually as r8ea2114 (presubmit successful).

[pajamallama, 2014-01-09 02:47:44]

Committed patchset #6 manually as rc5b7279 (presubmit successful).