Install pbs and pbs_ifl as loadable Python modules #1996

toonen · 2020-08-25T19:25:06Z

Describe Bug or Feature

These changes build and install the pbs and pbs_ifl Python modules as loadable shared library modules. This allows a standard Python 3.x interpreter to use either pbs or pbs_ifl, which is desirable for external Python programs that need to perform actions such as monitoring queued/running jobs or manipulate reservations. This also allows PTL tests to import either module to obtain constants defined by those modules and them in the validation portion of the test.

Describe Your Change

The loadable pbs_ifl module is generated from the same SWIG generated code that is statically incorporated in the PBS server and pbs_python code. The loadable pbs module is created in part by a new C module, src/modules/python/pbs_v1_module_init.c, that uses techniques similar to those used by pbs_python. Both modules are installed in %(libdir)/python/altair.

Attach Test and Valgrind Logs/Output

Two additional smoke tests were added. Please see test_import_pbs_module and test_import_pbs_ifl_module in test/tests/pbs_smoketest.py.

install-pbs-py-smoketest.log

CLAassistant · 2020-08-25T19:25:11Z

All committers have signed the CLA.

bayucan · 2020-08-28T21:56:49Z

src/modules/python/pbs_v1_module_init.c

+	return module;
+}
+
+/*


The following "dummy" variable declarations and function definitions are the same as the ones in the standalone src/tools/pbs_python.c. Please merge them somehow, so that there'll be only one place to update or add to, when new functions are introduced.

I agree. And my apologies for the delay in responding. I was on vacation for a couple of weeks and then playing catch-up. I will strip out the common code as much as seems reasonable and push an update once that is done.

I attempted to address the issue you raised about code duplication with the following commit: f548368. Please let me know if these changes are acceptable.

src/lib/Libifl/Makefile.am

test/fw/ptl.sh

src/modules/python/Makefile.am

bayucan · 2020-09-15T06:11:32Z

test/tests/pbs_smoketest.py

+        server_stat = pbs_ifl.pbs_statserver(server_conn, None, None)
+        pbs_ifl.pbs_disconnect(server_conn)
+        msg = "server name is %s" % (server_stat.name,)
+        self.logger.info(msg)


@checkmodule() test may not be reliable, so I suggest doing:

diff -c pbs_smoketest.py.orig pbs_smoketest.py *** pbs_smoketest.py.orig 2020-09-15 01:29:21.387629113 -0400 --- pbs_smoketest.py 2020-09-15 01:29:34.083629310 -0400 *************** *** 1463,1469 **** self.server.log_match("Req;req_reject;Reject reply code=15175", max_attempts=5) - @checkModule("pbs") def test_import_pbs_module(self): """ Test that the pbs module located in the PBS installation directory is --- 1463,1468 ---- *************** *** 1471,1481 **** the PYTHONPATH environment variable contain the path PBS_EXEC/{lib,lib64}/python/altair. """ import pbs msg = "pbs.JOB_STATE_RUNNING=%s" % (pbs.JOB_STATE_RUNNING,) self.logger.info(msg) - @checkModule("pbs_ifl") def test_import_pbs_ifl_module(self): """ Test that the pbs_ifl module located in the PBS installation directory --- 1470,1482 ---- the PYTHONPATH environment variable contain the path PBS_EXEC/{lib,lib64}/python/altair. """ + sys.path.append(os.path.join(self.server.pbs_conf['PBS_EXEC'], 'lib', + 'python', 'altair')) import pbs msg = "pbs.JOB_STATE_RUNNING=%s" % (pbs.JOB_STATE_RUNNING,) self.logger.info(msg) def test_import_pbs_ifl_module(self): """ Test that the pbs_ifl module located in the PBS installation directory *************** *** 1483,1488 **** --- 1484,1492 ---- This test requires the PYTHONPATH environment variable contain the path PBS_EXEC/{lib,lib64}/python/altair. """ + sys.path.append(os.path.join(self.server.pbs_conf['PBS_EXEC'], 'lib', + 'python', 'altair')) import pbs_ifl server_conn = pbs_ifl.pbs_connect(None) server_stat = pbs_ifl.pbs_statserver(server_conn, None, None)

I'm more than happy to make the changes you suggest; however, I must ask, if @checkModule() is unreliable. then shouldn't it be either fixed or eliminated. I ask because I used other test code that made use of this decorator as an example, so any issues with the decorator may extend beyond the tests I added.

@toonen : I would have to check what's going on with @checkModule. Maybe it's assuming that the environment where pbs_benchpress is invoked already contains path $PBS_EXEC/lib/python/altair. Maybe I didn't have that in my path when I invoked the testsuite, and it kept failing on @checkModule. When took out that check, and explictly added the path in the test case itself, it worked. This sounds more like a fail-safe way of running the test...

@bayucan: I applied the suggested patch in the following commit: 3a61204. Thanks for the suggestion.

This had the side effect of identifying an issue with -fsanitize=address, which is currently resulting in the import tests failing when that flag is included. I'm working on an autotools appropriate way to address the issue, which only reminds me that I'm a decade or so behind on frequent autotools use. :-)

Moved variable declarations and definitions, as well as stub function definitions, common to pbs_v1_module_init.c, pbs_python.c and pbsd_main.c into a common include file.

bayucan · 2020-09-24T16:49:13Z

test/tests/pbs_smoketest.py

@@ -1487,6 +1487,8 @@ def test_import_pbs_ifl_module(self):
        PBS_EXEC/{lib,lib64}/python/altair.
        """
        import pbs_ifl
+        sys.path.append(os.path.join(
+            self.server.pbs_conf['PBS_EXEC'], 'lib','python', 'altair'))


The 'import pbs_ifl' needs to go after the ..sys.append() otherwise we'll get an import error.

libtool was not passing the -fsanitize compiler options to the compiler when attempting to link shared libraries. this resulted in undefined symbol errors when the library was loaded by a program that hadn't been linked with those same -fsanitiize options.

bayucan · 2020-09-25T05:48:33Z

test/fw/ptl/utils/pbs_testsuite.py

+        for lib_dir in ['lib64', 'lib']:
+            pbs_python_path = os.path.join(self.server.pbs_conf['PBS_EXEC'],
+                lib_dir, 'python', 'altair')
+        if os.path.isdir(pbs_python_path):


Did you mean to indent this if statement to be under the for loop body?

Yes I did. Thanks for catching that! It's fixed in commit 5591dd0.

toonen · 2020-09-28T18:26:10Z

I believe I have addressed all of the issues that were pointed out. If I missed anything, please let me know.

bayucan

Looks good now! This is a really nice, useful, addition to PBS. Thanks.

src/include/pbs_v1_module_common.i

m4/pbs_patch_libtool.m4

subhasisb · 2020-09-29T05:30:17Z

m4/pbs_patch_libtool.m4

+			mv libtool libtool.orig
+			sed -e 's/\(-fuse-linker-plugin\)\([[|)]]\)/\1|-fsanitize=\*\2/' \
+				libtool.orig >libtool
+			rm -f libtool.orig


If the sed did something bad, we would be deleting the original libtool without checking the error status - perhaps we redirect sed output to libtool.upd (or similar name), check that the change we wanted is actually good and then mv libtool.upd to libtool when we are sure.

Good point. I made the modifications you suggested and also added code to generate an error file when something goes wrong. In addition, I changed the option used by sed to one that I hope is more common. The changes are in commit fa6afda.

src/include/pbs_v1_module_common.i

toonen added 2 commits August 19, 2020 16:15

Install pbs and pbs_ifl Python modules in PBS_EXEC

30e9793

Merge branch 'master' into install_pbs_ifl_py

bbfe6e3

toonen added 5 commits August 25, 2020 17:17

fixed warnings causing CI to fail

42f4de5

fixed issue with make dist

904e6e6

fixed missing libz in pbs python module

aaee7b9

hopeful fix for the continuing libz issues

3a3bcf5

probable fix for make dist not including SWIG input file

6358130

bayucan requested changes Aug 28, 2020

View reviewed changes

hirenvadalia requested changes Sep 5, 2020

View reviewed changes

src/lib/Libifl/Makefile.am Outdated Show resolved Hide resolved

test/fw/ptl.sh Show resolved Hide resolved

bayucan requested changes Sep 15, 2020

View reviewed changes

toonen added 7 commits September 21, 2020 19:58

Merge branch 'master' into install_pbs_py

ac324bf

Eliminated duplicate pbs v1 module code

f548368

Moved variable declarations and definitions, as well as stub function definitions, common to pbs_v1_module_init.c, pbs_python.c and pbsd_main.c into a common include file.

Changes provided by @bayucan to eliminate link failure on SLES12

cb037ab

removed use of checkModule on import tests

3a61204

Merge branch 'master' into install_pbs_py

bc04af6

fixed pep8 error

82d7dfe

added missing @swig_py_inc@ when generating pbs_ifl_wrap.c

da4d5c9

bayucan reviewed Sep 24, 2020

View reviewed changes

toonen added 4 commits September 24, 2020 16:47

add possible missing path to PBS Python modules

4860917

attempt to fix PEP8 issues

64c000e

another attempt to satisfy PEP8 and readability

eca9263

bayucan reviewed Sep 25, 2020

View reviewed changes

toonen added 2 commits September 28, 2020 11:56

fixed indentation in add_pbs_python_path_to_sys_path()

5591dd0

Merge branch 'master' into install_pbs_py

ff8b520

bayucan approved these changes Sep 28, 2020

View reviewed changes

hirenvadalia reviewed Sep 29, 2020

View reviewed changes

src/include/pbs_v1_module_common.i Show resolved Hide resolved

subhasisb requested changes Sep 29, 2020

View reviewed changes

toonen added 2 commits September 30, 2020 13:59

improved PBS_AC_PATCH_LIBTOOL

fa6afda

Merge branch 'master' into install_pbs_py

9b072c0

subhasisb approved these changes Oct 2, 2020

View reviewed changes

hirenvadalia approved these changes Oct 2, 2020

View reviewed changes

hirenvadalia merged commit f3c242a into openpbs:master Oct 2, 2020

suresh-thelkar pushed a commit to suresh-thelkar/openpbs that referenced this pull request Oct 7, 2020

Install pbs and pbs_ifl as loadable Python modules (openpbs#1996)

5335fb9

hirenvadalia mentioned this pull request Oct 13, 2020

Fix for NameError for pbs_connect in pbs_python #2073

Merged

toonen deleted the install_pbs_py branch April 16, 2021 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install pbs and pbs_ifl as loadable Python modules #1996

Install pbs and pbs_ifl as loadable Python modules #1996

toonen commented Aug 25, 2020

CLAassistant commented Aug 25, 2020 •

edited

Loading

bayucan Aug 28, 2020

toonen Sep 22, 2020

toonen Sep 22, 2020 •

edited

Loading

bayucan Sep 15, 2020

toonen Sep 22, 2020

bayucan Sep 22, 2020

toonen Sep 23, 2020

toonen Sep 24, 2020

bayucan Sep 24, 2020

bayucan Sep 25, 2020

toonen Sep 28, 2020

toonen commented Sep 28, 2020

bayucan left a comment •

edited

Loading

subhasisb Sep 29, 2020

toonen Sep 30, 2020

Install pbs and pbs_ifl as loadable Python modules #1996

Install pbs and pbs_ifl as loadable Python modules #1996

Conversation

toonen commented Aug 25, 2020

Describe Bug or Feature

Describe Your Change

Attach Test and Valgrind Logs/Output

CLAassistant commented Aug 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toonen Sep 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toonen commented Sep 28, 2020

bayucan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Aug 25, 2020 •

edited

Loading

toonen Sep 22, 2020 •

edited

Loading

bayucan left a comment •

edited

Loading