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 <russ...@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 <b...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
