Hi Stephen,

I've had a quick look at the patches labelled trivial.

For those patches en bloc:
Reviewed-by: Daniel Axtens <d...@axtens.net>

Regards,
Daniel

Stephen Finucane <stephenfinuc...@hotmail.com> 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 <stephenfinuc...@hotmail.com>
> ---
>  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
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to