Support pull directories #8
Conversation
| return dest_file.getvalue() | ||
|
|
||
| if dest_file: | ||
| print("pull: %s -> %s" % (device_filename, dest_file)) |
There was a problem hiding this comment.
No printing, please. Also, instead of an assert on 182, raise a custom error or something like ValueError that can be caught more specifically than the generic AssertionError.
|
Other than that one comment, LGTM. Thanks for adding that! |
|
Updated |
|
LGTM! I can't merge these myself, but hopefully @alusco is still paying attention these days |
|
|
||
| if stat.S_ISDIR(filemode): | ||
| if dest_file is None: | ||
| raise ValueError("Must specify dest_file when pulling a directory") |
There was a problem hiding this comment.
Can you add a check for dest_file being a directory (or not existing)? If they specify an existing file then we'll error out in a weird place down in Pull
There was a problem hiding this comment.
Alternatively, if they call without dest_file, can we return a map of StringIO objects?
dest_file = collections.defaultdict(cStringIO.StringIO)
...
self.Pull(
..., os.path.join(dest_file, device_file.filename)
if isinstance(dest_file, basestring) else dest_file[device_file.filename])then after the loop do:
if isinstance(dest_file, dict):
return {fn: data.getvalue() for fn, data in dest_file.iteritems()}
|
Any interest in this PR? I have write access now, but there are a few comments left alone here. If you get around to those, I can merge this PR. Otherwise, I'll take this over from you in late March |
google#8 wasn't merged yet.
Fix high_test after 65dec86
|
Ping? This PR had some activity, but none of the comments were addressed (and now it conflicts with master, sorry) |
No description provided.