-
Notifications
You must be signed in to change notification settings - Fork 44
added careful copy job with corresponding test. #424
Conversation
Can one of the admins verify this patch? |
Ok to test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes - then she goes in. Good
python/res/fm/shell/shell.py
Outdated
@staticmethod | ||
def carefulCopyFile(src , target = None): | ||
if os.path.exists(target): | ||
raise IOError("Input argument:'%s' already exist" % target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That the target file exists is perfectly OK - not exception.
python/res/fm/shell/shell.py
Outdated
@staticmethod | ||
def carefulCopyFile(src , target = None): | ||
if os.path.exists(target): | ||
raise IOError("Input argument:'%s' already exist" % target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That the target file exists is perfectly OK - not exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the function fail silently? Would it not be a good idea to let the user know if the copy failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well - if the file is already present the copy should not proceed, and that is the expected operation of this function - i.e. not a failure. Could add a message like:
print("File: {} already present - not updated".format(target_file))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good, I will change it accordingly.
@@ -0,0 +1,9 @@ | |||
STDOUT careful_copy_file.stdout | |||
STDERR careful_copy_file.stderr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The std file descriptors are redirected by default - not necessary to do it explicitly.
Printing warning when file exist Changed unit test of careful_copy_file accordingly
Task
#416
Approach
Short description of the approach
Pre un-WIP checklist
Depends on