From 1d784a819da5303419aed73ef328732f3299bd3e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 26 Jul 2024 15:49:16 -0500 Subject: [PATCH] fix: Make Java, Python wheel artifacts have same dependencies (#5850) Fixes #5848 --- py/embedded-server/build.gradle | 39 +++++++++++++++++++ py/embedded-server/java-runtime/build.gradle | 10 +++++ py/embedded-server/requirements-dev.txt | 10 +++++ .../tests/test_load_app_modules.py | 27 +++++++++++++ py/server/deephaven_internal/stream.py | 31 +++++++++------ server/jetty-app/build.gradle | 2 + server/netty-app/build.gradle | 6 +++ 7 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 py/embedded-server/requirements-dev.txt create mode 100644 py/embedded-server/tests/test_load_app_modules.py diff --git a/py/embedded-server/build.gradle b/py/embedded-server/build.gradle index d61f6f780ba..e0bc16be9db 100644 --- a/py/embedded-server/build.gradle +++ b/py/embedded-server/build.gradle @@ -3,6 +3,8 @@ plugins { id 'java-library' } +evaluationDependsOn ':docker-server-jetty' + configurations { classpath } @@ -24,3 +26,40 @@ dependencies { because 'downstream dagger compile' } } + +def testPyClient = Docker.registerDockerTask(project, 'testPyClient') { + copyIn { + from('tests') { + into 'project/tests' + } + from(tasks.getByName('buildWheel')) { + into 'wheels' + } + from(configurations.pythonWheel) { + into 'wheels' + } + from ('requirements-dev.txt') { + into 'project/' + } + } + dockerfile { + // Start from the image that built our wheel, so it is already ready to go + from(Docker.localImageName('server-jetty')) + copyFile('project', '/project') + copyFile('wheels', '/wheels') + + workingDir('/project') + runCommand '''set -eux; \\ + mkdir -p /out/report; \\ + pip install --upgrade pip; \\ + pip3 install -r requirements-dev.txt; \\ + pip3 install /wheels/*''' + } + parentContainers = [ project(':docker-server-jetty').tasks.findByName('buildDocker-server-jetty') ] + entrypoint = ['python', '-m', 'xmlrunner', 'discover', 'tests', '-v', '-o', '/out/report'] + copyOut { + into layout.buildDirectory.dir('test-results') + } +} + +tasks.getByName('check').dependsOn(testPyClient) diff --git a/py/embedded-server/java-runtime/build.gradle b/py/embedded-server/java-runtime/build.gradle index 4076cfde67e..00908428577 100644 --- a/py/embedded-server/java-runtime/build.gradle +++ b/py/embedded-server/java-runtime/build.gradle @@ -27,6 +27,10 @@ dependencies { runtimeOnly project(':hotspot-impl') } + if (!hasProperty('excludeClockImpl')) { + runtimeOnly project(':clock-impl') + } + if (!hasProperty('excludeSql')) { runtimeOnly project(':engine-sql') } @@ -35,6 +39,12 @@ dependencies { runtimeOnly project(':extensions-s3') runtimeOnly project(':extensions-iceberg-s3') } + + if (!hasProperty('excludeJson')) { + dependencies { + runtimeOnly project(':extensions-json-jackson') + } + } } // making a dir here isn't optimal, but without it we need to make py-embedded-server be a java and a python diff --git a/py/embedded-server/requirements-dev.txt b/py/embedded-server/requirements-dev.txt new file mode 100644 index 00000000000..5d9603fd665 --- /dev/null +++ b/py/embedded-server/requirements-dev.txt @@ -0,0 +1,10 @@ +pandas +pyarrow +grpcio +setuptools +protobuf +wheel +sphinx +bitstring +unittest-xml-reporting +timeout-decorator diff --git a/py/embedded-server/tests/test_load_app_modules.py b/py/embedded-server/tests/test_load_app_modules.py new file mode 100644 index 00000000000..e101c9f24d2 --- /dev/null +++ b/py/embedded-server/tests/test_load_app_modules.py @@ -0,0 +1,27 @@ +# +# Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +# + +import unittest +import importlib +import pkgutil +import sys + + +class LoadAllModules(unittest.TestCase): + def test_import_all_packages(self): + # First start the server that is defined in this package + from deephaven_server import Server + Server().start() + + # Then iterate through all deephaven packages to make sure they all can be imported + # using the wheel that we distribute + pkg = importlib.import_module('deephaven') + + mods = pkgutil.walk_packages(pkg.__path__, prefix='deephaven.') + for mod in mods: + if mod.name not in sys.modules: + try: + importlib.import_module(mod.name) + except: + ... diff --git a/py/server/deephaven_internal/stream.py b/py/server/deephaven_internal/stream.py index 846d5aeeaa9..afb6e1d4734 100644 --- a/py/server/deephaven_internal/stream.py +++ b/py/server/deephaven_internal/stream.py @@ -4,6 +4,22 @@ import io +def _get_encoding_or_utf8(stream) -> str: + """ + Helper to read the encoding from a stream. The stream implementation may or may not have an + encoding property, and if it has one it might be None - if absent or None, use utf8, otherwise + return the actual encoding value. + + Known cases: + - "no encoding attr' - jpy doesn't include this attr for its built-in stdout/stderr impls + - "encoding attr is None" - xmlrunner has an attribute, but it has a None value + - other cases seem to provide a real value. + :param stream: the stream to ask for its encoding + :return: the encoding to use + """ + return getattr(stream, 'encoding', 'UTF-8') or 'UTF-8' + + class TeeStream(io.TextIOBase): """TextIOBase subclass that splits output between a delegate instance and a set of lambdas. @@ -14,10 +30,7 @@ class TeeStream(io.TextIOBase): @classmethod def split(cls, py_stream, java_stream): - if hasattr(py_stream, "encoding"): - encoding = py_stream.encoding - else: - encoding = 'UTF-8' + encoding = _get_encoding_or_utf8(py_stream) return TeeStream( orig_stream=py_stream, should_write_to_orig_stream=True, @@ -28,10 +41,7 @@ def split(cls, py_stream, java_stream): @classmethod def redirect(cls, py_stream, java_stream): - if hasattr(py_stream, "encoding"): - encoding = py_stream.encoding - else: - encoding = 'UTF-8' + encoding = _get_encoding_or_utf8(py_stream) return TeeStream( orig_stream=py_stream, should_write_to_orig_stream=False, @@ -72,10 +82,7 @@ def fileno(self): @property def encoding(self): - if hasattr(self._stream, 'encoding') and self._stream.encoding is not None: - return self._stream.encoding - else: - return 'UTF-8' + return _get_encoding_or_utf8(self._stream) def write(self, string): self.write_func(string) diff --git a/server/jetty-app/build.gradle b/server/jetty-app/build.gradle index 5a74c1ce660..a9d06d63e35 100644 --- a/server/jetty-app/build.gradle +++ b/server/jetty-app/build.gradle @@ -64,6 +64,8 @@ if (!hasProperty('excludeJson')) { } } +// When you add a new dependency here, be sure to include it in server-netty-app and py-embedded-server + def authHandlers = [] def authConfigs = ['AuthHandlers'] if (hasProperty('anonymous')) { diff --git a/server/netty-app/build.gradle b/server/netty-app/build.gradle index f5891c28a57..c03899cc531 100644 --- a/server/netty-app/build.gradle +++ b/server/netty-app/build.gradle @@ -58,6 +58,12 @@ if (!hasProperty('excludeS3')) { } } +if (!hasProperty('excludeJson')) { + dependencies { + runtimeOnly project(':extensions-json-jackson') + } +} + def authHandlers = [] def authConfigs = ['AuthHandlers'] if (hasProperty('anonymous')) {