[ovs-dev] Remove Python 2 leftovers.

Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1949875
Signed-off-by: Rosemarie O'Riorden <roriorde@redhat.com>
---
 python/ovstest/tests.py          |  4 +---
 python/ovstest/util.py           |  2 +-
 python/setup.py                  |  2 --
 utilities/checkpatch.py          |  1 -
 utilities/ovs-l3ping.in          | 22 ++++++++++++----------
 utilities/ovs-parse-backtrace.in | 12 ++++++------
 utilities/ovs-pcap.in            |  4 +---
 utilities/ovs-vlan-test.in       | 28 ++++++++++++++--------------
 8 files changed, 35 insertions(+), 40 deletions(-)

On 6/8/21 12:31 AM, Rosemarie O'Riorden wrote:
> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1949875
> Signed-off-by: Rosemarie O'Riorden <roriorde@redhat.com>
> ---

Hi, Rosemarie.  Thanks for working on improving OVS!

See some comments inline.

Best regards, Ilya Maximets.

>  python/ovstest/tests.py          |  4 +---
>  python/ovstest/util.py           |  2 +-
>  python/setup.py                  |  2 --
>  utilities/checkpatch.py          |  1 -
>  utilities/ovs-l3ping.in          | 22 ++++++++++++----------
>  utilities/ovs-parse-backtrace.in | 12 ++++++------
>  utilities/ovs-pcap.in            |  4 +---
>  utilities/ovs-vlan-test.in       | 28 ++++++++++++++--------------
>  8 files changed, 35 insertions(+), 40 deletions(-)
> 
> diff --git a/python/ovstest/tests.py b/python/ovstest/tests.py
> index 6de3cc3af..a237ce16d 100644
> --- a/python/ovstest/tests.py
> +++ b/python/ovstest/tests.py
> @@ -10,12 +10,10 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>  
> -from __future__ import print_function

Quick grep over the OVS repository shows some other places where
print_function is imported from the __futire__.  Do we need to
remove these too?

$ git grep print_function
ofproto/ipfix-gen-entities:from __future__ import print_function
ovsdb/ovsdb-idlc.in:from __future__ import print_function
python/ovs/compat/sortedcontainers/sortedlist.py:from __future__ import print_function
tests/test-jsonrpc.py:from __future__ import print_function
tests/test-reconnect.py:from __future__ import print_function
utilities/gdb/ovs_gdb.py:from __future__ import print_function

> -
>  import math
>  import time
>  
> -import ovstest.util as util
> +import util

With this change in the virtual environment (described below)
I'm getting following errors:

python/ovstest/tests.py:27:14: E1101: Module 'util' has no 'rpc_client' member (no-member)
python/ovstest/tests.py:33:29: E1101: Module 'util' has no 'bandwidth_to_string' member (no-member)

I'd say that it picks up some different 'util' package.
With 'import ovstes.util as util' I don't see this problem.

>  
>  DEFAULT_TEST_BRIDGE = "ovstestbr0"
>  DEFAULT_TEST_PORT = "ovstestport0"
> diff --git a/python/ovstest/util.py b/python/ovstest/util.py
> index 72457158f..4caf6c352 100644
> --- a/python/ovstest/util.py
> +++ b/python/ovstest/util.py
> @@ -26,7 +26,7 @@ import socket
>  import struct
>  import subprocess
>  
> -import exceptions
> +import builtins as exceptions
>  
>  import xmlrpc.client
>  
> diff --git a/python/setup.py b/python/setup.py
> index d385d8372..a0f4ffded 100644
> --- a/python/setup.py
> +++ b/python/setup.py

I noticed that this file defines python2 compatibily in the
package description, e.g.:
  "Programming Language :: Python :: 2"
We should, probably, remove that, right?

> @@ -10,8 +10,6 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>  
> -from __future__ import print_function
> -
>  import sys
>  
>  from distutils.command.build_ext import build_ext
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index bc6bfae15..ac14da29b 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -13,7 +13,6 @@
>  # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
> -from __future__ import print_function
>  
>  import email
>  import getopt
> diff --git a/utilities/ovs-l3ping.in b/utilities/ovs-l3ping.in
> index 92d32acb3..89a57d529 100644
> --- a/utilities/ovs-l3ping.in
> +++ b/utilities/ovs-l3ping.in
> @@ -19,11 +19,13 @@ achieved by tunneling the control connection inside the tunnel itself.
>  """
>  
>  import socket
> -import xmlrpclib
> +import xmlrpc.client
>  
> -import ovstest.args as args
> -import ovstest.tests as tests
> -import ovstest.util as util
> +import sys
> +sys.path.insert(1, '../python/ovstest')
> +import args
> +import tests
> +import util

I think, these scripts expects that ovstest library is globally
available, e.g. by installing an openvswitch-test package or in
some other way.  Some of these files will also be part of the
package in the end and path ../python/ovstest will not be valid
on the end system.  You may simulate the end environment where
these scripts will be used with python virtual environment like
this:

python3 -m venv ./ovs-python-env # creating new virtual env
source ./ovs-python-env/bin/activate  # activating
cd ./python/

# installing ovs python package to the virtual env.
python3 setup.py install

# For the ovstest files there is no python-style installation
# procedure, so we're just copying files directly to the place
# where python will look for them.
cp -r ./ovstest ../ovs-python-env/lib/python3*/site-packages/
cd ..

At this point you have an environment where you can freely
import ovs and ovstest packages.  At this stage you can run
pylint or other tools and they should not complain about
missing imports.

deactivate # to deactivate the virtual env
rm -rf ./ovs-python-env  # to completely remove it.


In the end, I think that changes that replaces imports of
ovstest package are not necessary and might not work if
scripts are executed not from the OVS source directory.

>  
>  
>  def get_packet_sizes(me, he, remote_ip):
> @@ -64,13 +66,13 @@ if __name__ == '__main__':
>              ps = get_packet_sizes(me, he, args.client[0])
>              tests.do_direct_tests(me, he, bandwidth, interval, ps)
>      except KeyboardInterrupt:
> -        print "Terminating"
> -    except xmlrpclib.Fault:
> -        print "Couldn't contact peer"
> +        print("Terminating")
> +    except xmlrpc.client.Fault:
> +        print("Couldn't contact peer")
>      except socket.error:
> -        print "Couldn't contact peer"
> -    except xmlrpclib.ProtocolError:
> -        print "XMLRPC control channel was abruptly terminated"
> +        print("Couldn't contact peer")
> +    except xmlrpc.client.ProtocolError:
> +        print("XMLRPC control channel was abruptly terminated")
>      finally:
>          if local_server is not None:
>              local_server.terminate()
> diff --git a/utilities/ovs-parse-backtrace.in b/utilities/ovs-parse-backtrace.in
> index d5506769a..f44f05cd1 100755
> --- a/utilities/ovs-parse-backtrace.in
> +++ b/utilities/ovs-parse-backtrace.in
> @@ -70,7 +70,7 @@ result.  Expected usage is for ovs-appctl backtrace to be piped in.""")
>          if os.path.exists(debug):
>              binary = debug
>  
> -    print "Binary: %s\n" % binary
> +    print("Binary: %s\n" % binary)
>  
>      stdin = sys.stdin.read()
>  
> @@ -88,15 +88,15 @@ result.  Expected usage is for ovs-appctl backtrace to be piped in.""")
>      for lines, count in traces:
>          longest = max(len(l) for l in lines)
>  
> -        print "Backtrace Count: %d" % count
> +        print("Backtrace Count: %d" % count)
>          for line in lines:
>              match = re.search(r'\[(0x.*)]', line)
>              if match:
> -                print "%s %s" % (line.ljust(longest),
> -                                 addr2line(binary, match.group(1)))
> +                print("%s %s" % (line.ljust(longest),
> +                                 addr2line(binary, match.group(1))))
>              else:
> -                print line
> -        print
> +                print(line)
> +        print()
>  
>  
>  if __name__ == "__main__":
> diff --git a/utilities/ovs-pcap.in b/utilities/ovs-pcap.in
> index dddbee4df..6b5f63399 100755
> --- a/utilities/ovs-pcap.in
> +++ b/utilities/ovs-pcap.in
> @@ -14,8 +14,6 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>  
> -from __future__ import print_function
> -
>  import binascii
>  import getopt
>  import struct
> @@ -79,7 +77,7 @@ if __name__ == "__main__":
>          try:
>              options, args = getopt.gnu_getopt(sys.argv[1:], 'hV',
>                                                ['help', 'version'])
> -        except getopt.GetoptException as geo:
> +        except getopt.GetoptError as geo:
>              sys.stderr.write("%s: %s\n" % (argv0, geo.msg))
>              sys.exit(1)
>  
> diff --git a/utilities/ovs-vlan-test.in b/utilities/ovs-vlan-test.in
> index 154573a9b..6b906dc2d 100755
> --- a/utilities/ovs-vlan-test.in
> +++ b/utilities/ovs-vlan-test.in
> @@ -14,9 +14,9 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>  
> -import BaseHTTPServer
> +import http.server
>  import getopt
> -import httplib
> +import http.client

Please, re-sort the imports since names are different now.

>  import os
>  import threading
>  import time
> @@ -84,7 +84,7 @@ class UDPReceiver:
>  
>          try:
>              sock.bind((self.vlan_ip, self.vlan_port))
> -        except socket.error, e:
> +        except socket.error as e:
>              print_safe('Failed to bind to %s:%d with error: %s'
>                      % (self.vlan_ip, self.vlan_port, e))
>              os._exit(1) #sys.exit only exits the current thread.
> @@ -95,7 +95,7 @@ class UDPReceiver:
>                  data, _ = sock.recvfrom(4096)
>              except socket.timeout:
>                  continue
> -            except socket.error, e:
> +            except socket.error as e:
>                  print_safe('Failed to receive from %s:%d with error: %s'
>                      % (self.vlan_ip, self.vlan_port, e))
>                  os._exit(1)
> @@ -180,7 +180,7 @@ class VlanServer:
>              for _ in range(send_time * 2):
>                  try:
>                      send_packet(test_id, size, ip, port)
> -                except socket.error, e:
> +                except socket.error as e:
>                      self.set_result(test_id, 'Failure: ' + str(e))
>                      return
>                  time.sleep(.5)
> @@ -194,15 +194,15 @@ class VlanServer:
>      def run(self):
>          self.udp_recv.start()
>          try:
> -            BaseHTTPServer.HTTPServer((self.server_ip, self.server_port),
> +            http.server.HTTPServer((self.server_ip, self.server_port),
>                      VlanServerHandler).serve_forever()
> -        except socket.error, e:
> +        except socket.error as e:
>              print_safe('Failed to start control server: %s' % e)
>              self.udp_recv.stop()
>  
>          return 1
>  
> -class VlanServerHandler(BaseHTTPServer.BaseHTTPRequestHandler):
> +class VlanServerHandler(http.server.BaseHTTPRequestHandler):
>      def do_GET(self):
>  
>          #Guarantee three arguments.
> @@ -244,7 +244,7 @@ class VlanClient:
>          self.udp_recv       = UDPReceiver(vlan_ip, vlan_port)
>  
>      def request(self, resource):
> -        conn = httplib.HTTPConnection(self.server_ip_port)
> +        conn = http.client.HTTPConnection(self.server_ip_port)
>          conn.request('GET', resource)
>          return conn
>  
> @@ -256,7 +256,7 @@ class VlanClient:
>          try:
>              conn = self.request('/start/recv')
>              data = conn.getresponse().read()
> -        except (socket.error, httplib.HTTPException), e:
> +        except (socket.error, http.client.HTTPException) as e:
>              error_msg(e)
>              return False
>  
> @@ -277,7 +277,7 @@ class VlanClient:
>                  send_packet(test_id, size, ip, port)
>                  resp = self.request('/result/%d' % test_id).getresponse()
>                  data = resp.read()
> -            except (socket.error, httplib.HTTPException), e:
> +            except (socket.error, http.client.HTTPException) as e:
>                  error_msg(e)
>                  return False
>  
> @@ -302,7 +302,7 @@ class VlanClient:
>          try:
>              conn    = self.request(resource)
>              test_id = conn.getresponse().read()
> -        except (socket.error, httplib.HTTPException), e:
> +        except (socket.error, http.client.HTTPException) as e:
>              error_msg(e)
>              return False
>  
> @@ -335,7 +335,7 @@ class VlanClient:
>          try:
>              resp = self.request('/ping').getresponse()
>              data = resp.read()
> -        except (socket.error, httplib.HTTPException), e:
> +        except (socket.error, http.client.HTTPException) as e:
>              error_msg(e)
>              return False
>  
> @@ -383,7 +383,7 @@ def main():
>      try:
>          options, args = getopt.gnu_getopt(sys.argv[1:], 'hVs',
>                                            ['help', 'version', 'server'])
> -    except getopt.GetoptError, geo:
> +    except getopt.GetoptError as geo:
>          print_safe('%s: %s\n' % (sys.argv[0], geo.msg))
>          return 1
>  
>