ZOOKEEPER-3790: zkpython compilation and testing issues

This series makes the zkpython "contrib" compile cleanly, and makes the tests runnable out of the box with Python 3:

  * Defined `THREADED`, as zkpython uses the sync API

    Without this, compilation produces a number of warnings about undefined functions (and misleading suggestions!) as it only sees the async API:

        src/c/zookeeper.c:1080:13: warning: implicit declaration of function 'zoo_delete'; did you mean 'zoo_adelete'? [-Wimplicit-function-declaration]
           int err = zoo_delete(zh, path, version);
                     ^~~~~~~~~~
                     zoo_adelete

  * Define `HAVE_OPENSSL_H`, as the extension calls zookeeper_init_ssl

    The flag is unconditionally defined for now, as the function is unconditionally called.

        src/c/zookeeper.c:646:10: warning: implicit declaration of function 'zookeeper_init_ssl'; did you mean 'zookeeper_init2'? [-Wimplicit-function-declaration]
             zh = zookeeper_init_ssl( host, cert_str, watcherfn != Py_None ? watcher_dispatch : NULL,

                  ^~~~~~~~~~~~~~~~~~
                  zookeeper_init2

  * Make SSL support optional (but on by default)

  * Raise `MemoryError` if module initialization fails

  * Allow for version/ABI information in shared object name

    In some versions of the Python framework, native extensions encode the interpreter version and some ABI information in the filename, giving e.g. `zookeeper.cpython-37m-x86_64-linux-gnu.so` instead of `zookeeper.so`.  Take this into account when setting up test runs.

  * Make sure test failures are detected

  * `async` is a keyword in Python 3.5+

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Mate Szalay-Beko <szalay.beko.mate@gmail.com>, Norbert Kalmar <nkalmar@apache.org>

Closes #1312 from ztzg/ZOOKEEPER-3790-zkpython-compilation-and-testing-issues

(cherry picked from commit 21221ac692405962bdda3e9ceb63186ef0f180ef)
Signed-off-by: Norbert Kalmar <nkalmar@apache.org>

zookeeper-contrib/zookeeper-contrib-zkpython/README

@@ -5,13 +5,15 @@ Please do not rely on APIs staying constant in the short term. The handling of e
 DEPENDENCIES:
 -------------
 
-This has only been tested against SVN (i.e. 3.2.0 in development) but should work against 3.1.1. 
+This has only been tested against SVN/Git (i.e. 3.2.0 in development) but should work against 3.1.1.
 
 You will need the Python development headers installed to build the module - on many package-management systems, these can be found in python-devel. (On ubuntu 18.4, install python2.7 and python2.7-dev.)
 
-Python >= 2.6 is required. We have tested against 2.6. We have not tested against 3.x. 
+Python >= 2.6 is required. We have tested against 2.6 and 3.5+.
 
-E.g. setting up tpyhon and python devel on ubuntu 18.4:
+By default, the extension assumes that the C client library was compiled with OpenSSL enabled (--with-openssl).  You can disable OpenSSL support in the Python binding by setting the ZKPYTHON_NO_SSL environment variable to a non-empty string before executing Ant or setup.py.
+
+E.g. setting up python and python devel on ubuntu 18.4:
 sudo apt-get install python2.7 python2.7-dev
 sudo update-alternatives --install /usr/bin/python python /usr/bin/python2.7 1
 
@@ -22,7 +24,7 @@ To install, make sure that the C client has been built (use `mvn clean install -
 
 ant install
 
-from zookeeper/src/contrib/zkpython/.
+from zookeeper-contrib/zookeeper-contrib-zkpython/.
 
 To test, run ant test from the same directory. 
 

zookeeper-contrib/zookeeper-contrib-zkpython/src/c/zookeeper.c

@@ -600,7 +600,7 @@ void acl_completion_dispatch(int rc, struct ACL_vector *acl, struct Stat *stat,
 /* -------------------------------------------------------------------------- */
 
 
-static PyObject *pyzookeeper_init_optional_ssl(PyObject *self, PyObject *args, int ssl) {
+static PyObject *pyzookeeper_init_common(PyObject *self, PyObject *args, int ssl) {
   const char *host;
   const char *cert_str;
   PyObject *watcherfn = Py_None;
@@ -643,8 +643,13 @@ static PyObject *pyzookeeper_init_optional_ssl(PyObject *self, PyObject *args, i
   watchers[handle] = pyw;
 
   if (ssl) {
+#ifdef HAVE_OPENSSL_H
     zh = zookeeper_init_ssl( host, cert_str, watcherfn != Py_None ? watcher_dispatch : NULL,
                              recv_timeout, cid.client_id == -1 ? 0 : &cid, pyw, 0 );
+#else
+    fprintf(stderr, "SSL support not compiled in (called with ssl=%d).\n", ssl);
+    abort();
+#endif
   } else {
     zh = zookeeper_init( host, watcherfn != Py_None ? watcher_dispatch : NULL,
                          recv_timeout, cid.client_id == -1 ? 0 : &cid, pyw, 0 );
@@ -652,7 +657,7 @@ static PyObject *pyzookeeper_init_optional_ssl(PyObject *self, PyObject *args, i
 
   if (zh == NULL)
     {
-      PyErr_SetString( ZooKeeperException, "Could not internally obtain SSL zookeeper handle" );
+      PyErr_Format( ZooKeeperException, "Could not internally obtain%s zookeeper handle", ssl ? " SSL" : "" );
       return NULL;
     }
 
@@ -662,14 +667,16 @@ static PyObject *pyzookeeper_init_optional_ssl(PyObject *self, PyObject *args, i
 
 static PyObject *pyzookeeper_init(PyObject *self, PyObject *args)
 {
-  return pyzookeeper_init_optional_ssl(self, args, 0);
+  return pyzookeeper_init_common(self, args, /*ssl*/0);
 }
 
 
+#ifdef HAVE_OPENSSL_H
 static PyObject *pyzookeeper_init_ssl(PyObject *self, PyObject *args)
 {
-  return pyzookeeper_init_optional_ssl(self, args, 1);
+  return pyzookeeper_init_common(self, args, /*ssl*/1);
 }
+#endif
 
 
 /* -------------------------------------------------------------------------- */
@@ -1518,7 +1525,9 @@ PyObject *pyzoo_deterministic_conn_order(PyObject *self, PyObject *args)
 
 static PyMethodDef ZooKeeperMethods[] = {
   {"init", pyzookeeper_init, METH_VARARGS, pyzk_init_doc },
+#ifdef HAVE_OPENSSL_H
   {"init_ssl", pyzookeeper_init_ssl, METH_VARARGS, pyzk_init_ssl_doc },
+#endif
   {"create",pyzoo_create, METH_VARARGS, pyzk_create_doc },
   {"delete",pyzoo_delete, METH_VARARGS, pyzk_delete_doc },
   {"get_children", pyzoo_get_children, METH_VARARGS, pyzk_get_children_doc },
@@ -1589,8 +1598,14 @@ PyMODINIT_FUNC initzookeeper(void) {
 #else
   PyObject *module = Py_InitModule("zookeeper", ZooKeeperMethods);
 #endif
+
   if (init_zhandles(32) == 0) {
-    return; // TODO: Is there any way to raise an exception here?
+#if PY_MAJOR_VERSION >= 3
+    Py_DECREF(module);
+    return PyErr_NoMemory();
+#else
+    return;
+#endif
   }
 
   ZooKeeperException = PyErr_NewException("zookeeper.ZooKeeperException",

zookeeper-contrib/zookeeper-contrib-zkpython/src/python/setup.py

@@ -15,11 +15,20 @@
 # limitations under the License.
 
 from distutils.core import setup, Extension
+import os
 
 zookeeper_basedir = "../../"
 
+zookeeper_macros = [("THREADED", None)]
+
+# Assume the C extension includes OpenSSL support unless told
+# otherwise.
+if not os.environ.get("ZKPYTHON_NO_SSL"):
+    zookeeper_macros.append(("HAVE_OPENSSL_H", True))
+
 zookeepermodule = Extension("zookeeper",
                             sources=["src/c/zookeeper.c"],
+                            define_macros=zookeeper_macros,
                             include_dirs=[zookeeper_basedir + "/zookeeper-client/zookeeper-client-c/include",
                                           zookeeper_basedir + "/zookeeper-client/zookeeper-client-c/target/c",
                                           zookeeper_basedir + "/zookeeper-client/zookeeper-client-c/generated"],

zookeeper-contrib/zookeeper-contrib-zkpython/src/test/async_test.py

@@ -26,7 +26,7 @@ class AsyncTest(zktestbase.TestBase):
 
     def test_async(self):
         self.assertEqual(self.connected, True)
-        ret = zookeeper.async(self.handle, "/")
+        ret = getattr(zookeeper, 'async')(self.handle, "/")
         self.assertEqual(ret, zookeeper.OK, "async failed")
         
 if __name__ == '__main__':

zookeeper-contrib/zookeeper-contrib-zkpython/src/test/callback_test.py

@@ -91,9 +91,9 @@ class CallbackTest(zktestbase.TestBase):
                                                                self.create_callback( dispatch_callback )),
                                lambda: self.assertEqual(True, self.callback_flag, "Strings dispatch not fired"))
 
-        self.callback_harness( lambda: zookeeper.async(self.handle,
-                                                       "/",
-                                                       self.create_callback( dispatch_callback )),
+        self.callback_harness( lambda: getattr(zookeeper, 'async')(self.handle,
+                                                                   "/",
+                                                                   self.create_callback( dispatch_callback )),
                                lambda: self.assertEqual(True, self.callback_flag, "String dispatch not fired"))
 
         self.callback_harness( lambda: zookeeper.aget_acl(self.handle,

zookeeper-contrib/zookeeper-contrib-zkpython/src/test/connection_test.py

@@ -58,7 +58,8 @@ class ConnectionTest(zktestbase.TestBase):
                           self.handle,
                           "/")
 
-
+    @unittest.skipUnless(hasattr(zookeeper, 'init_ssl'),
+                         "SSL support not compiled in.")
     def testsslconnection(self):
         cv = threading.Condition()
         self.connected = False

zookeeper-contrib/zookeeper-contrib-zkpython/src/test/run_tests.sh

@@ -19,6 +19,8 @@
 # Usage: run_tests.sh testdir [logdir]
 # logdir is optional, defaults to cwd
 
+set -e
+
 # get the number of command-line arguments given
 ARGC=$#
 
@@ -30,7 +32,7 @@ else
 fi
 
 # Find the build directory containing zookeeper.so
-SO_PATH=`find ./target/ -name "zookeeper.so" | head -1`
+SO_PATH=`find ./target/ -name 'zookeeper*.so' | head -1`
 PYTHONPATH=`dirname $SO_PATH`
 LIB_PATH=../../zookeeper-client/zookeeper-client-c/target/c/.libs
 for test in `ls $1/*_test.py`;