Message ID | 20190523182622.386876-1-shekhar250198@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Pablo Neira |
Headers | show |
Series | [nft,v4] tests: py: fix python3 | expand |
On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > This version of the patch converts the file into python3 and also uses > .format() method to make the print statments cleaner. > > The version history of this topic is: > > v1: conversion to py3 by changing print statements. > v2: adds the '__future__' package for compatibility with py2 and py3. > v3: solves the 'version' problem in argparse by adding a new argument. > v4: uses .format() method to make the print statements cleaner. > > > Signed-off-by: Shekhar Sharma <shekhar250198@gmail.com> > --- Acked-by: Eric Garver <eric@garver.life> > tests/py/nft-test.py | 47 ++++++++++++++++++++++++-------------------- > 1 file changed, 26 insertions(+), 21 deletions(-) > > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py > index 1c0afd0e..ab26d08d 100755 > --- a/tests/py/nft-test.py > +++ b/tests/py/nft-test.py [..] > @@ -1353,15 +1358,15 @@ def main(): > signal.signal(signal.SIGTERM, signal_handler) > > if os.getuid() != 0: > - print "You need to be root to run this, sorry" > + print("You need to be root to run this, sorry") > return > > # Change working directory to repository root > os.chdir(TESTS_PATH + "/../..") > > if not os.path.exists('src/.libs/libnftables.so'): > - print "The nftables library does not exist. " \ > - "You need to build the project." > + print("The nftables library does not exist. " \ > + "You need to build the project.") nit: The trailing '\' can be removed now that the strings are inside parenthesis. I don't think it's worth rerolling the patch though.
On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > This version of the patch converts the file into python3 and also uses > .format() method to make the print statments cleaner. Applied, thanks.
On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > This version of the patch converts the file into python3 and also uses > > .format() method to make the print statments cleaner. > > Applied, thanks. Hm. I'm hitting this here after applying this: # python nft-test.py Traceback (most recent call last): File "nft-test.py", line 17, in <module> from nftables import Nftables ImportError: No module named nftables
On Fri, May 24, 2019 at 09:46:05PM +0200, Pablo Neira Ayuso wrote: > On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > > This version of the patch converts the file into python3 and also uses > > > .format() method to make the print statments cleaner. > > > > Applied, thanks. > > Hm. > > I'm hitting this here after applying this: > > # python nft-test.py > Traceback (most recent call last): > File "nft-test.py", line 17, in <module> > from nftables import Nftables > ImportError: No module named nftables Did you build nftables --with-python-bin ? The error can occur if you built nftables against a different python version. e.g. built for python3, but the "python" executable is python2.
On Tue, May 28, 2019 at 09:32:06AM -0400, Eric Garver wrote: > On Fri, May 24, 2019 at 09:46:05PM +0200, Pablo Neira Ayuso wrote: > > On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > > > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > > > This version of the patch converts the file into python3 and also uses > > > > .format() method to make the print statments cleaner. > > > > > > Applied, thanks. > > > > Hm. > > > > I'm hitting this here after applying this: > > > > # python nft-test.py > > Traceback (most recent call last): > > File "nft-test.py", line 17, in <module> > > from nftables import Nftables > > ImportError: No module named nftables > > Did you build nftables --with-python-bin ? The error can occur if you > built nftables against a different python version. e.g. built for > python3, but the "python" executable is python2. Actually, it's probably caused by this hunk: @@ -13,6 +13,8 @@ # Thanks to the Outreach Program for Women (OPW) for sponsoring this test # infrastructure. +from __future__ import print_function +from nftables import Nftables import sys import os import argparse @@ -22,7 +24,6 @@ import json TESTS_PATH = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/')) -from nftables import Nftables TESTS_DIRECTORY = ["any", "arp", "bridge", "inet", "ip", "ip6"] LOGFILE = "/tmp/nftables-test.log" I don't know why the import of nftables was moved. But it was moved to _before_ the modification of the import search path (sys.path.insert()). Moving it back should fix the issue. Sorry I missed it.
On Tue, May 28, 2019, 7:09 PM Eric Garver <eric@garver.life> wrote: > > On Tue, May 28, 2019 at 09:32:06AM -0400, Eric Garver wrote: > > On Fri, May 24, 2019 at 09:46:05PM +0200, Pablo Neira Ayuso wrote: > > > On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > > > > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > > > > This version of the patch converts the file into python3 and also uses > > > > > .format() method to make the print statments cleaner. > > > > > > > > Applied, thanks. > > > > > > Hm. > > > > > > I'm hitting this here after applying this: > > > > > > # python nft-test.py > > > Traceback (most recent call last): > > > File "nft-test.py", line 17, in <module> > > > from nftables import Nftables > > > ImportError: No module named nftables > > > > Did you build nftables --with-python-bin ? The error can occur if you > > built nftables against a different python version. e.g. built for > > python3, but the "python" executable is python2. > > Actually, it's probably caused by this hunk: > > @@ -13,6 +13,8 @@ > # Thanks to the Outreach Program for Women (OPW) for sponsoring this test > # infrastructure. > > +from __future__ import print_function > +from nftables import Nftables > import sys > import os > import argparse > @@ -22,7 +24,6 @@ import json > TESTS_PATH = os.path.dirname(os.path.abspath(__file__)) > sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/')) > > -from nftables import Nftables > > TESTS_DIRECTORY = ["any", "arp", "bridge", "inet", "ip", "ip6"] > LOGFILE = "/tmp/nftables-test.log" > > I don't know why the import of nftables was moved. But it was moved to > _before_ the modification of the import search path (sys.path.insert()). > Moving it back should fix the issue. Sorry I missed it. Yes, I think the problem is resolved now. I shouldn't have moved it to the top. Thanks! Shekhar
On Tue, May 28, 2019 at 09:32:06AM -0400, Eric Garver wrote: > On Fri, May 24, 2019 at 09:46:05PM +0200, Pablo Neira Ayuso wrote: > > On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > > > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > > > This version of the patch converts the file into python3 and also uses > > > > .format() method to make the print statments cleaner. > > > > > > Applied, thanks. > > > > Hm. > > > > I'm hitting this here after applying this: > > > > # python nft-test.py > > Traceback (most recent call last): > > File "nft-test.py", line 17, in <module> > > from nftables import Nftables > > ImportError: No module named nftables > > Did you build nftables --with-python-bin ? The error can occur if you > built nftables against a different python version. e.g. built for > python3, but the "python" executable is python2. Thanks for explaining. When running: ./configure --help it shows this: --enable-python Enable python If I use it, I get this: nft configuration: cli support: yes enable debugging symbols: yes use mini-gmp: no enable man page: yes libxtables support: yes json output support: yes enable Python: yes (with yes) <------ $ make ... setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py ... (forever loop) so it indeed uses 'yes' :-) same effect in case I specify --with-python-bin with no path, ie. ./configure --with-python-bin --with-xtables --enable-python --with-json Thanks!
On Wed, May 29, 2019 at 09:37:02AM +0200, Pablo Neira Ayuso wrote: > On Tue, May 28, 2019 at 09:32:06AM -0400, Eric Garver wrote: > > On Fri, May 24, 2019 at 09:46:05PM +0200, Pablo Neira Ayuso wrote: > > > On Fri, May 24, 2019 at 09:36:00PM +0200, Pablo Neira Ayuso wrote: > > > > On Thu, May 23, 2019 at 11:56:22PM +0530, Shekhar Sharma wrote: > > > > > This version of the patch converts the file into python3 and also uses > > > > > .format() method to make the print statments cleaner. > > > > > > > > Applied, thanks. > > > > > > Hm. > > > > > > I'm hitting this here after applying this: > > > > > > # python nft-test.py > > > Traceback (most recent call last): > > > File "nft-test.py", line 17, in <module> > > > from nftables import Nftables > > > ImportError: No module named nftables > > > > Did you build nftables --with-python-bin ? The error can occur if you > > built nftables against a different python version. e.g. built for > > python3, but the "python" executable is python2. > > Thanks for explaining. > > When running: > > ./configure --help > > it shows this: > > --enable-python Enable python > > If I use it, I get this: > > nft configuration: > cli support: yes > enable debugging symbols: yes > use mini-gmp: no > enable man page: yes > libxtables support: yes > json output support: yes > enable Python: yes (with yes) <------ > > $ make > ... > setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py > setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py > setup.py build --build-base /home/pablo/devel/scm/git-netfilter/nftables/py > ... > (forever loop) > > so it indeed uses 'yes' :-) > > same effect in case I specify --with-python-bin with no path, ie. > > ./configure --with-python-bin --with-xtables --enable-python --with-json It was found in another thread that the error you see is caused by an issue in the patch. That being said.. --with-python-bin is to specify the interpreter. So you need to pass it something like /usr/bin/python2 or /usr/bin/python3. By default configure will autodetect - I think it prefers python2. e.g. $ ./configure --enable-python --with-python-bin=/usr/bin/python3
diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py index 1c0afd0e..ab26d08d 100755 --- a/tests/py/nft-test.py +++ b/tests/py/nft-test.py @@ -13,6 +13,8 @@ # Thanks to the Outreach Program for Women (OPW) for sponsoring this test # infrastructure. +from __future__ import print_function +from nftables import Nftables import sys import os import argparse @@ -22,7 +24,6 @@ import json TESTS_PATH = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/')) -from nftables import Nftables TESTS_DIRECTORY = ["any", "arp", "bridge", "inet", "ip", "ip6"] LOGFILE = "/tmp/nftables-test.log" @@ -436,7 +437,7 @@ def set_delete(table, filename=None, lineno=None): ''' Deletes set and its content. ''' - for set_name in all_set.keys(): + for set_name in list(all_set.keys()): # Check if exists the set if not set_exist(set_name, table, filename, lineno): reason = "The set %s does not exist, " \ @@ -1002,9 +1003,9 @@ def execute_cmd(cmd, filename, lineno, stdout_log=False, debug=False): :param debug: temporarily set these debug flags ''' global log_file - print >> log_file, "command: %s" % cmd + print("command: {}".format(cmd), file=log_file) if debug_option: - print cmd + print(cmd) if debug: debug_old = nftables.get_debug() @@ -1198,7 +1199,7 @@ def run_test_file(filename, force_all_family_option, specific_file): sys.stdout.flush() if signal_received == 1: - print "\nSignal received. Cleaning up and Exitting..." + print("\nSignal received. Cleaning up and Exitting...") cleanup_on_exit() sys.exit(0) @@ -1305,13 +1306,13 @@ def run_test_file(filename, force_all_family_option, specific_file): if specific_file: if force_all_family_option: - print print_result_all(filename, tests, total_warning, total_error, - total_unit_run) + print(print_result_all(filename, tests, total_warning, total_error, + total_unit_run)) else: - print print_result(filename, tests, total_warning, total_error) + print(print_result(filename, tests, total_warning, total_error)) else: if tests == passed and tests > 0: - print filename + ": " + Colors.GREEN + "OK" + Colors.ENDC + print(filename + ": " + Colors.GREEN + "OK" + Colors.ENDC) f.close() del table_list[:] @@ -1322,7 +1323,7 @@ def run_test_file(filename, force_all_family_option, specific_file): def main(): - parser = argparse.ArgumentParser(description='Run nft tests', version='1.0') + parser = argparse.ArgumentParser(description='Run nft tests') parser.add_argument('filenames', nargs='*', metavar='path/to/file.t', help='Run only these tests') @@ -1341,6 +1342,10 @@ def main(): dest='enable_json', help='test JSON functionality as well') + parser.add_argument('-v', '--version', action='version', + version= '1.0', + help='prints the version info.') + args = parser.parse_args() global debug_option, need_fix_option, enable_json_option debug_option = args.debug @@ -1353,15 +1358,15 @@ def main(): signal.signal(signal.SIGTERM, signal_handler) if os.getuid() != 0: - print "You need to be root to run this, sorry" + print("You need to be root to run this, sorry") return # Change working directory to repository root os.chdir(TESTS_PATH + "/../..") if not os.path.exists('src/.libs/libnftables.so'): - print "The nftables library does not exist. " \ - "You need to build the project." + print("The nftables library does not exist. " \ + "You need to build the project.") return global nftables @@ -1411,18 +1416,18 @@ def main(): run_total += file_unit_run if test_files == 0: - print "No test files to run" + print("No test files to run") else: if not specific_file: if force_all_family_option: - print "%d test files, %d files passed, %d unit tests, " \ - "%d total executed, %d error, %d warning" \ - % (test_files, files_ok, tests, run_total, errors, - warnings) + print("{} test files, {} files passed, {} unit tests, " \ + "{} total executed, {} error, {} warning"\ + .format(test_files, files_ok, tests, run_total, errors, + warnings)) else: - print "%d test files, %d files passed, %d unit tests, " \ - "%d error, %d warning" \ - % (test_files, files_ok, tests, errors, warnings) + print("{} test files, {} files passed, {} unit tests, " \ + "{} error, {} warning"\ + .format(test_files, files_ok, tests, errors, warnings)) if __name__ == '__main__':
This version of the patch converts the file into python3 and also uses .format() method to make the print statments cleaner. The version history of this topic is: v1: conversion to py3 by changing print statements. v2: adds the '__future__' package for compatibility with py2 and py3. v3: solves the 'version' problem in argparse by adding a new argument. v4: uses .format() method to make the print statements cleaner. Signed-off-by: Shekhar Sharma <shekhar250198@gmail.com> --- tests/py/nft-test.py | 47 ++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 21 deletions(-)