-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://fedorahosted.org/reviewboard/r/304/#review481
-----------------------------------------------------------
src/rolekit/server/io/rolesettings.py
<
https://fedorahosted.org/reviewboard/r/304/#comment1017>
Given that this is now a full path instead of just a filename, it would probably be a
good idea to rename this to self.filepath
src/rolekit/server/io/rolesettings.py
<
https://fedorahosted.org/reviewboard/r/304/#comment1018>
Checking for os.path.exists() here is redundant and faulty. You should just do try:
os.remove(self.filename)
except OSError:
pass
The original method contained a race-condition where if the file was removed by
another process between the exists() and the remove(), we'd have an unhandled
exception.
- Stephen Gallagher
On July 10, 2014, 12:08 p.m., Thomas Woerner wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://fedorahosted.org/reviewboard/r/304/
-----------------------------------------------------------
(Updated July 10, 2014, 12:08 p.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
-------
Simplified src/rolekit/server/io/rolesettings.RoleSettings, new remove method
Diffs
-----
src/rolekit/server/io/rolesettings.py 671a40a36be363a4d0e8f9446dc4e1c2277cbcd5
Diff:
https://fedorahosted.org/reviewboard/r/304/diff/
Testing
-------
Thanks,
Thomas Woerner