Code review comment for lp:~rackspace-titan/nova/osapi-pass-update-lp744567

Revision history for this message
Jay Pipes (jaypipes) wrote :

Hi!

19 + if not isinstance(name, basestring) or name == '':
20 + return exc.HTTPBadRequest()

Not going to weigh in on Brian Lamar's question, but I would say that if you stick with raising an HTTPBadRequest in this way, that you should, at a minimum, provide an error message. I'd hate to receive a standard 400 Bad Request and have no idea why ;)

something like this would work:

if not isinstance(name, basestring):
    msg = _("Name supplied for server (%r) is not a string or unicode") % name
    return exc.HTTPBadRequest(msg)
elif name.strip() == '':
    msg = _("Null string supplied as server name")
    return exc.HTTPBadRequest(msg)

Other than that nit, looks good, and good work adding tests.

Cheers!
jay

review: Needs Fixing

« Back to merge proposal