0

This is a python script that runs in a Django environment by Celery. I need to create a catch the 'rest of the errors' and raise an exception so Celery will send a email on that exception.

Would this be the best way?

for thread in thread_batch:
   try:
        obj = query_set.get(thread_id=thread['thread_id'])
        for key, value in thread.iteritems():
            setattr(obj, key, value)
        obj.unanswered = True
    except ThreadVault.DoesNotExist:
        obj = ThreadVault(**thread)
    except:
        raise Exception("There has been a unknown error in the database")
    obj.save()
2
  • Seems fine to me. The last except does exactly what you described you want to have happen. Although I wonder why you wouldn't let Celery catch the original error anyway in that case. Commented Jun 6, 2014 at 17:37
  • @dman Answer improved. Commented Jun 6, 2014 at 17:55

1 Answer 1

1

Yes, and empty except will catch any exception different from ThreadVault.DoesNotExist (in this case). But you can improve your code a little more.

Try always to put the less code possible inside the try block. You code could be:

for thread in thread_batch:
    try:
        obj = query_set.get(thread_id=thread['thread_id'])
    except ThreadVault.DoesNotExist:
        obj = ThreadVault(**thread)
    except:
        raise Exception("There has been a unknown error in the database")
    else:    # Note we add the else statement here.
        for key, value in thread.iteritems():
            setattr(obj, key, value)
        obj.unanswered = True
    # Since save function also hits the database
    # it should be within a try block as well.
    try:
       obj.save()
    except:
       raise Exception("There has been a unknown error in the database")
Sign up to request clarification or add additional context in comments.

4 Comments

The problem that I have with this construct and the specific need of OP is that in a hypothetical situation where an error is raised within the else, the original intent is thwarted. The situation is unlikely in this scenario, but let's imagine that a couple of months from now someone adds some line after obj.unanswered = True.
@mike I don't think so, take a look at the "generic exception" thrown by the OP. raise Exception("There has been a unknown error in the database") so it is obvious that all he's interested in catch are database errors. The only line that hit the database is obj = query_set.get(thread_id=thread['thread_id']) hence, the other code can go perfectly in a separate else block, without affect the OP intentions.
@mike the other line hitting the database is obj.save() it should put that line within a try block too.
your code is not bad practice and my reserve is not based solely upon the message in the exception that is thrown. It's rather about the intent specified in the description of the question I need to create a catch the 'rest of the errors'. The way I see it, he just wants a catchall that will send an email. The message in the email is more or less relevant, it says very little other than "there was an error somewhere, go check".

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.