The "plus-plus" views are actually specific to the "contact" widget, move them there.
rebased
For PEP8 imports are ordered by standard library, then third-party libraries, then the current package:
from __future__ import unicode_literals import json from mock import mock # I *think* this can just be `import mock` based on its usage below import flask from hubs.... import hubs
Not a big deal, but it makes it easy to see what's coming from where at a glance.
Did you mean 404 here?
Does flask not handle unhandled exceptions with an HTTP 500 anyway?
You probably want to add a timeout to this class. It's also worth noting using requests this way won't take advantage of connection pooling. You'll need to create a thread-local session for that.
I think this should be a 404 as well.
Should this be a 400 since it's a bad request?
Pretty much the same comments as the above requests try/except block.
1 new commit added
I did all that, except the request connection pooling which probably needs to be at a higher level to be useful to all widgets. By the way, Mailman has a different policy on the import order, that's why I got mixed up. Thanks for your review.
Sorry I forgot to mention this during my initial review, but if you wanted to be fancy here, you could do
try: req = requests.get(pp_url, timeout=5) except requests.Timeout: return 'The request to {ur} timed out'.format(url=pp_url), 504
That way the client knows the upstream service was the problem.
@abompard, one more comment I didn't think to make first time round (sorry!), and I went ahead and filed an issue about setting up a session. I'll probably take that issue on today.
Oh, that's a good idea, thanks :-)
Cool, looks good to me!
Pull-Request has been merged by abompard
The "plus-plus" views are actually specific to the "contact" widget, move them there.