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

Simplified src/rolekit/server/io/rolesettings.RoleSettings, new remove method

Diffs

  • src/rolekit/server/io/rolesettings.py (671a40a36be363a4d0e8f9446dc4e1c2277cbcd5)

View Diff