Code review comment for lp:~mew/charm-helpers/fetchallthethings

Revision history for this message
Liam Young (gnuoy) wrote :

I had a few issues when giving this a test run:

1) s/os.path.environ.get/os.environ.get/
2) install_remote expects a directory to be passed back by handler.install but the extract* classes don't return that in the archive class
3) For a tar ball the open method needs to be called on the tar ball and the extract all method is named differently i.e. This works for zip but not tar:
    archive = archive_class(archive_name)
    archive.extract_all(destpath)
This works for tar but not zip:
    archive = archive_class.open(archive_name)
    archive.extractall(destpath)
4) In the archive plugin the source archive needs to be passed when calling extract, i.e. it should be extract(url_parts.path, dest_file) not extract(dest_file)

review: Needs Fixing

« Back to merge proposal