Hi Stephen, I've had a quick look at the patches labelled trivial.
For those patches en bloc: Reviewed-by: Daniel Axtens <[email protected]> Regards, Daniel Stephen Finucane <[email protected]> writes: > The are two somewhat significant changes: > > * The behavior of 'Bundle.add_patch' is changed. Previously this would > raise an exception if the provided patch already existed in the > bundle. Since this code was only used in one location, change this to > return the BundlePatch if valid else None and change the calling code > to check return value instead of catching the exception. > * Use a context manager to open the config file in pwclient. This loses > a little granularity in error messaging, but this is a worthy > compromise. > > Signed-off-by: Stephen Finucane <[email protected]> > --- > patchwork/bin/pwclient | 11 ++--------- > patchwork/models.py | 4 +++- > patchwork/notifications.py | 3 ++- > patchwork/paginator.py | 4 ++-- > patchwork/templatetags/listurl.py | 6 +++--- > patchwork/views/__init__.py | 6 +++--- > patchwork/views/bundle.py | 15 ++++----------- > patchwork/views/mail.py | 4 ++-- > patchwork/views/patch.py | 14 +++++++------- > patchwork/views/user.py | 4 ++-- > patchwork/views/xmlrpc.py | 6 +++--- > 11 files changed, 33 insertions(+), 44 deletions(-) > > diff --git a/patchwork/bin/pwclient b/patchwork/bin/pwclient > index 791c511..b63db53 100755 > --- a/patchwork/bin/pwclient > +++ b/patchwork/bin/pwclient > @@ -294,17 +294,10 @@ def action_get(rpc, patch_id): > fname = "%s.%d" % (base_fname, i) > i += 1 > > - try: > - f = open(fname, "w") > - except: > - sys.stderr.write("Unable to open %s for writing\n" % fname) > - sys.exit(1) > - > - try: > + with open(fname, 'w') as f: > f.write(unicode(s).encode("utf-8")) > - f.close() > print('Saved patch to %s' % fname) > - except: > + except IOError: > sys.stderr.write("Failed to write to %s\n" % fname) > sys.exit(1) > > diff --git a/patchwork/models.py b/patchwork/models.py > index b1c6d8d..4c0aa88 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -580,12 +580,14 @@ class Bundle(models.Model): > # see if the patch is already in this bundle > if BundlePatch.objects.filter(bundle=self, > patch=patch).count(): > - raise Exception('patch is already in bundle') > + return > > bp = BundlePatch.objects.create(bundle=self, patch=patch, > order=max_order + 1) > bp.save() > > + return bp > + > def public_url(self): > if not self.public: > return None > diff --git a/patchwork/notifications.py b/patchwork/notifications.py > index 5420401..16152ef 100644 > --- a/patchwork/notifications.py > +++ b/patchwork/notifications.py > @@ -19,6 +19,7 @@ > > import datetime > import itertools > +import smtplib > > from django.conf import settings > from django.contrib.auth.models import User > @@ -87,7 +88,7 @@ def send_notifications(): > > try: > message.send() > - except Exception as ex: > + except smtplib.SMTPException as ex: > errors.append((recipient, ex)) > continue > > diff --git a/patchwork/paginator.py b/patchwork/paginator.py > index e31c76c..3da2cd0 100644 > --- a/patchwork/paginator.py > +++ b/patchwork/paginator.py > @@ -55,9 +55,9 @@ class Paginator(paginator.Paginator): > super(Paginator, self).__init__(objects, items_per_page) > > try: > - page_no = int(request.GET.get('page')) > + page_no = int(request.GET.get('page', 1)) > self.current_page = self.page(int(page_no)) > - except Exception: > + except ValueError: > page_no = 1 > self.current_page = self.page(page_no) > > diff --git a/patchwork/templatetags/listurl.py > b/patchwork/templatetags/listurl.py > index 3f28f71..eea788e 100644 > --- a/patchwork/templatetags/listurl.py > +++ b/patchwork/templatetags/listurl.py > @@ -63,9 +63,9 @@ class ListURLNode(template.defaulttags.URLNode): > > params = [] > try: > - qs_var = template.Variable('list_view.params') > - params = dict(qs_var.resolve(context)) > - except Exception: > + qs_var = template.Variable('list_view.params').resolve(context) > + params = dict(qs_var) > + except TypeError, template.VariableDoesNotExist: > pass > > for (k, v) in self.params.items(): > diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py > index e1e40a8..98e451d 100644 > --- a/patchwork/views/__init__.py > +++ b/patchwork/views/__init__.py > @@ -173,13 +173,13 @@ def set_bundle(request, project, action, data, patches, > context): > try: > bp = BundlePatch.objects.get(bundle=bundle, patch=patch) > bp.delete() > + except BundlePatch.DoesNotExist: > + pass > + else: > messages.success( > request, > "Patch '%s' removed from bundle %s\n" % (patch.name, > bundle.name)) > - # TODO(stephenfin): Make this less broad > - except Exception: > - pass > > bundle.save() > > diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py > index ba569e2..e717429 100644 > --- a/patchwork/views/bundle.py > +++ b/patchwork/views/bundle.py > @@ -48,10 +48,7 @@ def setbundle(request): > patch_id = request.POST.get('patch_id', None) > if patch_id: > patch = get_object_or_404(Patch, id=patch_id) > - try: > - bundle.append_patch(patch) > - except Exception: > - pass > + bundle.append_patch(patch) > bundle.save() > elif action == 'add': > bundle = get_object_or_404(Bundle, > @@ -66,11 +63,8 @@ def setbundle(request): > patch_ids = get_patch_ids(request.POST) > > for patch_id in patch_ids: > - try: > - patch = Patch.objects.get(id=patch_id) > - bundle.append_patch(patch) > - except: > - pass > + patch = Patch.objects.get(id=patch_id) > + bundle.append_patch(patch) > > bundle.save() > elif action == 'delete': > @@ -78,11 +72,10 @@ def setbundle(request): > bundle = Bundle.objects.get(owner=request.user, > id=request.POST['id']) > bundle.delete() > - except Exception: > + except Bundle.DoesNotExist: > pass > > bundle = None > - > else: > bundle = get_object_or_404(Bundle, owner=request.user, > id=request.POST['bundle_id']) > diff --git a/patchwork/views/mail.py b/patchwork/views/mail.py > index 3695e5b..8744d79 100644 > --- a/patchwork/views/mail.py > +++ b/patchwork/views/mail.py > @@ -17,7 +17,7 @@ > # along with Patchwork; if not, write to the Free Software > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > -from __future__ import absolute_import > +import smtplib > > from django.conf import settings as conf_settings > from django.core.mail import send_mail > @@ -114,7 +114,7 @@ def _optinout(request, action, description): > send_mail('Patchwork %s confirmation' % description, mail, > conf_settings.DEFAULT_FROM_EMAIL, [email]) > context['email_sent'] = True > - except Exception: > + except smtplib.SMTPException: > context['error'] = ('An error occurred during confirmation . ' > 'Please try again later.') > context['admins'] = conf_settings.ADMINS > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py > index 41a2ec8..e477572 100644 > --- a/patchwork/views/patch.py > +++ b/patchwork/views/patch.py > @@ -77,15 +77,15 @@ def patch(request, patch_id): > elif action == 'addtobundle': > bundle = get_object_or_404( > Bundle, id=request.POST.get('bundle_id')) > - try: > - bundle.append_patch(patch) > - bundle.save() > + if bundle.append_patch(patch) > messages.success(request, > - 'Patch added to bundle "%s"' % bundle.name) > - except Exception as ex: > + 'Patch "%s" added to bundle "%s"' % ( > + patch.name, bundle.name)) > + else: > messages.error(request, > - "Couldn't add patch '%s' to bundle %s: %s" > - % (patch.name, bundle.name, ex.message)) > + 'Failed to add patch "%s" to bundle "%s": ' > + 'patch is already in bundle' % ( > + patch.name, bundle.name)) > > # all other actions require edit privs > elif not editable: > diff --git a/patchwork/views/user.py b/patchwork/views/user.py > index dfbfde8..b82636f 100644 > --- a/patchwork/views/user.py > +++ b/patchwork/views/user.py > @@ -17,7 +17,7 @@ > # along with Patchwork; if not, write to the Free Software > # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > > -from __future__ import absolute_import > +import smtplib > > from django.contrib import auth > from django.contrib.auth.decorators import login_required > @@ -145,7 +145,7 @@ def link(request): > context, request=request), > settings.DEFAULT_FROM_EMAIL, > [form.cleaned_data['email']]) > - except Exception: > + except smtplib.SMTPException: > context['confirmation'] = None > context['error'] = ('An error occurred during confirmation. ' > 'Please try again later') > diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py > index ea2a1e3..793e82a 100644 > --- a/patchwork/views/xmlrpc.py > +++ b/patchwork/views/xmlrpc.py > @@ -94,7 +94,7 @@ class PatchworkXMLRPCDispatcher(SimpleXMLRPCDispatcher, > try: > decoded = > base64.b64decode(header.encode('ascii')).decode('ascii') > username, password = decoded.split(':', 1) > - except: > + except ValueError: > raise Exception('Invalid authentication credentials') > > return authenticate(username=username, password=password) > @@ -124,7 +124,7 @@ class PatchworkXMLRPCDispatcher(SimpleXMLRPCDispatcher, > response = self.dumps(response, methodresponse=1) > except six.moves.xmlrpc_client.Fault as fault: > response = self.dumps(fault) > - except: > + except Exception: # noqa > # report exception back to server > response = self.dumps( > six.moves.xmlrpc_client.Fault( > @@ -149,7 +149,7 @@ def xmlrpc(request): > if request.method == 'POST': > try: > ret = dispatcher._marshaled_dispatch(request) > - except Exception: > + except Exception: # noqa > return HttpResponseServerError() > else: > ret = dispatcher.generate_html_documentation() > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > [email protected] > https://lists.ozlabs.org/listinfo/patchwork
signature.asc
Description: PGP signature
_______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
