Edward Haas has posted comments on this change.
Change subject: utils: atomic write ......................................................................
Patch Set 7: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/61482/7/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 827: """ Line 828: return os.times()[4] Line 829: Line 830: Line 831: def random_iface_name(prefix='', max_length=15): Suggestion: Rename this to something generic, like random_name.
Then call it instead of _random_alnum... We do not need the _random_alnum... it give no readability or reuse now.
The docstring needs to change as well.
I guess you will need a separate patch for this... sorry. Line 832: """ Line 833: Create a network device name with the supplied prefix and a pseudo-random Line 834: suffix, e.g. dummy_ilXaYiSn7. The name is bound to IFNAMSIZ of 16-1 chars. Line 835: """
https://gerrit.ovirt.org/#/c/61482/7/tests/utilsTests.py File tests/utilsTests.py:
PS7, Line 1070: original_text='foo', new_text='bar' It will be nicer if you can arrange them as in the func definition.
Line 1068: Line 1069: def test_edit_file(self): Line 1070: self._test_atomic_write(original_text='foo', new_text='bar') Line 1071: Line 1072: def _test_atomic_write(self, new_text, original_text=None): Too much logic exists here with regard to original_text existing or not.
It may be nicer and clearer to do the actions in the test. Line 1073: with namedTemporaryDir() as tmp_dir: Line 1074: test_file_path = os.path.join(tmp_dir, 'foo.txt') Line 1075: if original_text is not None: Line 1076: with open(test_file_path, 'w') as f: