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.

-- 
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to