From 17aa6b844dea5d202649d13ba4bc87e6bbb70aa6 Mon Sep 17 00:00:00 2001
From: Branson Stephens <stephenb@uwm.edu>
Date: Fri, 3 May 2013 16:38:32 -0500
Subject: [PATCH] added some basic error handling for log entries that fail to
save because of IntegrityError
---
gracedb/api.py | 27 +++++++++++++++++++++---
gracedb/models.py | 29 +++++++++++++++++++------
gracedb/views.py | 54 +++++++++++++++++++++++++++++++++++++----------
3 files changed, 89 insertions(+), 21 deletions(-)
diff --git a/gracedb/api.py b/gracedb/api.py
index f2722cb60..364996cb5 100644
--- a/gracedb/api.py
+++ b/gracedb/api.py
@@ -421,6 +421,12 @@ class EventDetail(APIView):
handle_uploaded_data(event, uploadDestination)
event.submitter = request.ligouser
except:
+ # XXX Bad news. If the log file fails to save because of
+ # race conditions, then this will also be the the message
+ # returned. Somehow, I think there are other things that
+ # could go wrong inside handle_uploaded_data besides just
+ # bad data. We should probably check for different types
+ # of exceptions here.
return Response("Bad Data",
status=status.HTTP_400_BAD_REQUEST)
return Response(status=status.HTTP_202_ACCEPTED)
@@ -593,7 +599,12 @@ class EventLogList(APIView):
comment=message)
logset = event.eventlog_set.order_by("created")
n = len(logset)
- logentry.save()
+ try:
+ logentry.save()
+ except Exception as e:
+ # Since this is likely due to race conditions, we will return 503
+ return Response("Failed to save log entry: %s" % str(e),
+ status=status.HTTP_503_SERVICE_UNAVAILABLE)
rv = eventLogToDict(logentry, n, request=request)
response = Response(rv, status=status.HTTP_201_CREATED)
response['Location'] = rv['self']
@@ -843,7 +854,12 @@ class EventLogTagDetail(APIView):
logentry = EventLog(event=event,
issuer=request.ligouser,
comment=msg)
- logentry.save()
+ try:
+ logentry.save()
+ except Exception as e:
+ # Since the tag creation was successful, we'll return 201.
+ return Response("Tag succesful, but failed to create log entry: %s" % str(e),
+ status=status.HTTP_201_CREATED)
return Response("Tag created.",status=status.HTTP_201_CREATED)
@@ -872,7 +888,12 @@ class EventLogTagDetail(APIView):
logentry = EventLog(event=event,
issuer=request.ligouser,
comment=msg)
- logentry.save()
+ try:
+ logentry.save()
+ except Exception as e:
+ # Since the tag creation was successful, we'll return 200.
+ return Response("Tag removed, but failed to create log entry: %s" % str(e),
+ status=status.HTTP_200_OK)
return Response("Tag deleted.",status=status.HTTP_200_OK)
except:
diff --git a/gracedb/models.py b/gracedb/models.py
index 93fe079a0..df009db73 100644
--- a/gracedb/models.py
+++ b/gracedb/models.py
@@ -1,4 +1,4 @@
-from django.db import models
+from django.db import models, IntegrityError
from django.core.urlresolvers import reverse
from model_utils.managers import InheritanceManager
@@ -220,12 +220,27 @@ class EventLog(models.Model):
return logset.index(self)
def save(self, *args, **kwargs):
- if self.event.eventlog_set.count():
- self.N = event.eventlog_set.order_by('-N')[0].N + 1
- else:
- self.N = 1
- # XXX This call to save should be inside a try/except. Loop or something.
- super(EventLog, self).save(*args, **kwargs)
+ success = False
+ attempts = 0
+ while (not success and attempts < 5):
+ attempts = attempts + 1
+ if self.event.eventlog_set.count():
+ self.N = event.eventlog_set.order_by('-N')[0].N + 1
+ else:
+ self.N = 1
+ try:
+ super(EventLog, self).save(*args, **kwargs)
+ success = True
+ except IntegrityError:
+ # IntegrityError means an attempt to insert a duplicate
+ # key or to violate a foreignkey constraint.
+ # We are under race conditions. Let's try again.
+ pass
+
+ if not success:
+ # XXX Should this be a custom exception? That way we could catch it
+ # in the views that use it and give an informative error message.
+ raise Exception("Too many attempts to save log message. Something is wrong.")
class Labelling(models.Model):
event = models.ForeignKey(Event)
diff --git a/gracedb/views.py b/gracedb/views.py
index b78030ca3..98497b827 100644
--- a/gracedb/views.py
+++ b/gracedb/views.py
@@ -121,7 +121,12 @@ def skyalert(request, graceid):
if createLogEntry:
logentry = EventLog(event=event, issuer=request.ligouser, comment=message)
- logentry.save()
+ try:
+ logentry.save()
+ except:
+ # XXX Failed to create log entry for skyalert submission.
+ # Error message?
+ pass
return HttpResponseRedirect(reverse(view, args=[graceid]))
@@ -298,13 +303,16 @@ def _createLog(request, graceid, comment, uploadedFile=None):
logEntry.filename = uploadedFile.name
except Exception, e:
rdict['error'] = "Problem saving file: %s" % str(e)
- logEntry.save()
+ try:
+ logEntry.save()
- if request.POST.get('alert') == "True":
- description = "LOG: "
- if uploadedFile:
- description = "UPLOAD: '%s' " % uploadedFile.name
- issueAlertForUpdate(event, description+comment, doxmpp=True, filename=uploadedFile.name)
+ if request.POST.get('alert') == "True":
+ description = "LOG: "
+ if uploadedFile:
+ description = "UPLOAD: '%s' " % uploadedFile.name
+ issueAlertForUpdate(event, description+comment, doxmpp=True, filename=uploadedFile.name)
+ except Exception, e:
+ rdict['error'] = "Failed to save log message: %s" % str(e)
# XXX should be json
rval = str(rdict)
@@ -397,7 +405,11 @@ def create_label(graceid, labelName, creator, doAlert=True, doXMPP=True):
labelling.save()
message = "Label: %s" % label.name
log = EventLog(event=event, issuer=creator, comment=message)
- log.save()
+ try:
+ log.save()
+ except Exception as e:
+ # XXX This looks a bit odd to me.
+ d['error'] = str(e)
try:
issueAlertForLabel(event, label, doXMPP)
@@ -451,7 +463,12 @@ def logentry(request, graceid, num=None):
# create a log entry
elog = EventLog(event=event, issuer=request.ligouser)
elog.comment = request.POST.get('comment') or request.GET.get('comment')
- elog.save()
+ try:
+ elog.save()
+ except Exception as e:
+ # XXX I feel like this should be a 500 error.
+ return HttpResponse("ERROR: %s" % str(e))
+
tagname = request.POST.get('tagname')
if tagname:
# Look for the tag. If it doesn't already exist, create it.
@@ -469,7 +486,16 @@ def logentry(request, graceid, num=None):
tlog = EventLog(event=event,
issuer=request.ligouser,
comment=msg)
- tlog.save()
+ try:
+ tlog.save()
+ except Exception as e:
+ # XXX Maybe this isn't a big deal. It's more of a
+ # warning than an error.
+ msg = "Failed to save log entry to document tag: "
+ msg = msg + str(e)
+ msg = msg + "\n However, the log message itself was saved."
+ return HttpResponse(msg)
+
else:
try:
elog = event.eventlog_set.order_by('created').all()[int(num)]
@@ -1050,7 +1076,13 @@ def taglogentry(request, graceid, num, tagname):
logentry = EventLog(event=event,
issuer=request.ligouser,
comment=msg)
- logentry.save()
+ try:
+ logentry.save()
+ except Exception as e:
+ msg = "Failed to save log entry documenting tag: "
+ msg = msg + str(e) + '\n'
+ msg = "The tag itself, however, is saved."
+ return HttpResponse(msg, content_type="text")
else:
# We will only allow PUT here. Anything else is a bad request: 400
return HttpResponseBadRequest
--
GitLab