Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle IOError exceptions #4

Closed
wants to merge 2 commits into from
Closed

Handle IOError exceptions #4

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7d7d39
Handle IOError exceptions
Jun 16, 2016
8db2970
Minor changes after PR review. Bump version to 2.0.0
Jul 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions supervise.py
View file
Open in desktop
Expand Up @@ -267,9 +267,13 @@ def stop(self):
self._signal("d")

def _signal(self, signal):
f = open(self._control,"w+")
f.write(signal)
f.close()
try:
f = open(self._control,"w+")
f.write(signal)
except IOError as e:
raise ServiceControlError("Unable to write service control file '{0}'".format(e.filename))
else:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of sketchy because of the write fails, we still want to close the file.

Probably initialize f = None outside of the try block and do a conditional close in a finally:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean something like this?

f = None
try:
    f = open(self._control,"w+")
    f.write(signal)
except IOError as e:
    if f is not None:
      try:
        f.close()
       except IOError:
        pass
    raise ServiceControlError("Unable to write service control file '{0}'".format(e.filename))

What about splitting the 2 operations in different try/exc blocks?

try:
    f = open(self._control,"w+")
except IOError as e:
    raise ServiceControlError("Unable to open service control file '{0}'".format(e.filename))
try:
    f.write(signal)
except IOError as e:
    raise ServiceControlError("Unable to write service control file '{0}'".format(e.filename))
finally:
    f.close()

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking:

f = None
try:
    f = open(...)
    f.write(...)
except IOError as e:
    raise SuperviseControlError(...)
finally:
    if f is not None:
        f.close()

Splitting and the extra try around the close is probably overkill, but I'm not used to weird scenarios that you can't close fds on.

f.close()

def status(self):
""" Read the status of a service using status binary form. Returns
Expand All @@ -289,14 +293,21 @@ def status(self):
reached (means downtime for STATUS_DOWN).

"""
s = open(self._status,"rb").read(20)
try:
l = open(self._status,"rb")
s = l.read(20)
except IOError as e:
raise ServiceStatusError("Unable to read service status file '{0}'".format(e.filename))
else:
l.close()

if len(s) == 18:
seconds, nano, pid, paused, want = struct.unpack(">qllbc", s)
term, finish = 0, 0
elif len(s) == 20:
seconds, nano, pid, paused, want, term, finish = struct.unpack(">qllbcbb", s)
else:
raise AssertionError("Unknown status format")
raise ServiceStatusError("Unknown status format")

# pid is returned little-endian. Flip it.
pid, = struct.unpack("<l", struct.pack(">l", pid))
Expand Down Expand Up @@ -329,3 +340,10 @@ def status(self):
seconds = 0 if now < seconds else (now - seconds)

return ServiceStatus(status=status, pid=pid, action=action, uptime=seconds)


class ServiceStatusError(Exception):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a shared 'SuperviseError' base class. s/Service/Supervise/ for consistency with the module name.

''' An error occurred while reading service status file'''

class ServiceControlError(Exception):
''' An error occurred while writing service control file'''