Yaniv Bronhaim has posted comments on this change.
Change subject: utils: atomic file write ......................................................................
Patch Set 9: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/61482/9/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 932: return rget(dict.get(keys[0]), keys[1:], default) Line 933: Line 934: Line 935: @contextmanager Line 936: def atomic_file_write(file, flag): file is a saved word.. better to use file_name.
I think 'w' should be the default flag for file_write... or even hardcoded flag.. why do you need to accept here different flags at all? Line 937: """Atomically write into a file. Line 938: Line 939: Usage: Line 940:
Line 933: Line 934: Line 935: @contextmanager Line 936: def atomic_file_write(file, flag): Line 937: """Atomically write into a file. I don't know why in rget above the doc string is in that format, but please break the line after the """ prefix as in the rest of the comments """ docs """ Line 938: Line 939: Usage: Line 940: Line 941: with atomic_write('foo.txt', 'w') as f:
Line 942: f.write('shrubery') Line 943: # there are no changes on foo.txt yet Line 944: # now it is changed Line 945: """ Line 946: _, tmp_file = tempfile.mkstemp(dir=os.path.dirname(os.path.abspath(file)), you can't ignore the fd. you need to close it Line 947: prefix=os.path.basename(file), Line 948: suffix='.tmp') Line 949: try: Line 950: if os.path.exists(file):