[ovs-dev] [PATCH 02/11] python: Remove unused imports and variables.

[Ben Pfaff]

On Tue, Jan 05, 2016 at 01:30:39PM -0500, Russell Bryant wrote:
> On 01/04/2016 07:26 PM, Ben Pfaff wrote:
> > On Tue, Dec 22, 2015 at 12:17:24PM -0500, Russell Bryant wrote:
> >> This resolves the following flake8 error types:
> >>
> >>   F841 local variable 'e' is assigned to but never used
> >>   F401 'exceptions' imported but unused
> >>
> >> Signed-off-by: Russell Bryant <russell at ovn.org>
> > 
> > Do these "try" statements actually do anything?  It looks like they
> > immediately re-raise the exceptions they catch.  Is that just a no-op?
> 
> Yes, it's a no-op.  I'll remove them.
> 
> >> @@ -302,12 +301,12 @@ def set_dscp(sock, family, dscp):
> >>      if family == socket.AF_INET:
> >>          try:
> >>              sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
> >> -        except socket.error, e:
> >> +        except socket.error:
> >>              raise
> >>      elif family == socket.AF_INET6:
> >>          try:
> >>              sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
> >> -        except socket.error, e:
> >> +        except socket.error:
> >>              raise
> >>      else:
> >>          raise
> 
> and now that I look here again, this last line is a bug.  It needs to
> include an exception type since it's not re-raising from within an
> exception handler.  I'm going to go ahead and fix it while I'm changing
> this function anyway.  Here's the incremental diff.
> 
> > diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
> > index 7b943cf..04fb6af 100644
> > --- a/python/ovs/socket_util.py
> > +++ b/python/ovs/socket_util.py
> > @@ -299,14 +299,8 @@ def set_dscp(sock, family, dscp):
> >  
> >      val = dscp << 2
> >      if family == socket.AF_INET:
> > -        try:
> > -            sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
> > -        except socket.error:
> > -            raise
> > +        sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
> >      elif family == socket.AF_INET6:
> > -        try:
> > -            sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
> > -        except socket.error:
> > -            raise
> > +        sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
> >      else:
> > -        raise
> > +        raise ValueError('Invalid family %d' % family)
> 
> The function already raises ValueError in the case of an invalid value
> for dscp, so it seems like a reasonable choice.
> 
> > 
> > Acked-by: Ben Pfaff <blp at ovn.org>
> 
> Thanks!  I'm adding your ACK to the patch with the above incremental
> diff applied.

Thanks.  I think this is a nice bug fix and improvement.  Perhaps it
should be backported.