This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/304/ |
src/rolekit/server/io/rolesettings.py (Diff revision 1) | |||
---|---|---|---|
41 | self.read() |
41 | self.filename = "%s/%s.json" % (self.path, self.name) |
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 (Diff revision 1) | |||
---|---|---|---|
67 | def remove(self): |
||
68 | if os.path.exists(self.filename): |
||
69 | os.remove(self.filename) |
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 10th, 2014, 12:08 p.m. EDT, Thomas Woerner wrote:
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
By Thomas Woerner.
Updated July 10, 2014, 12:08 p.m.
Repository:
rolekit
Description
Diffs
|