From c5db1de7a658469177df269b42c993683dee4e64 Mon Sep 17 00:00:00 2001 From: Lennart Regebro Date: Tue, 23 Jul 2024 17:55:31 +0200 Subject: [PATCH] Added an "info()" method to the server, and a retrying that uses it. (#124) * Added an "info()" method to the server, and a retrying that uses it. * Verify filters on clientside * More logging * Updated changes --------- Co-authored-by: Lennart Regebro --- CHANGES.rst | 11 ++++ src/unoserver/client.py | 102 +++++++++++++++++++++++++++++++++++++- src/unoserver/server.py | 31 ++++++++++-- tests/test_integration.py | 21 ++++---- tests/test_server.py | 1 + 5 files changed, 150 insertions(+), 16 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 335f3e4..673b8cb 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -3,6 +3,17 @@ - ReeceJones added support to specify IPv6 adresses. +- Now tries to connect to the server, with retries if the server has + not been started yet. + +- Verifies that the version installed on the server and client is the same. + +- If you misspell a filter name, the output is nicer. + +- The clients got very silent in the refactor, fixed that. + +- --verbose and --quiet arguments to get even more output, or less. + 2.1 (2024-03-26) ---------------- diff --git a/src/unoserver/client.py b/src/unoserver/client.py index f20f3c3..4538f8c 100644 --- a/src/unoserver/client.py +++ b/src/unoserver/client.py @@ -2,6 +2,7 @@ import logging import os import sys +import time from importlib import metadata from xmlrpc.client import ServerProxy @@ -41,6 +42,29 @@ def __init__(self, server="127.0.0.1", port="2003", host_location="auto"): else: raise RuntimeError("host_location can be 'auto', 'remote', or 'local'") + def _connect(self, proxy, retries=5, sleep=10): + """Check the connection to the proxy multiple times + + Returns the info() data (unoserver version + filters)""" + + while retries > 0: + try: + info = proxy.info() + if not info["unoserver"] == __version__: + raise RuntimeError( + f"Version mismatch. Client runs {__version__} while " + f"Server runs {info['unoserver']}" + ) + return info + except ConnectionError as e: + logger.debug(f"Error {e.strerror}, waiting...") + retries -= 1 + if retries > 0: + time.sleep(sleep) + logger.debug("Retrying...") + else: + raise + def convert( self, inpath=None, @@ -95,6 +119,25 @@ def convert( raise ValueError("The outpath can not be a directory") with ServerProxy(f"http://{self.server}:{self.port}", allow_none=True) as proxy: + logger.info("Connecting.") + logger.debug(f"Host: {self.server} Port: {self.port}") + info = self._connect(proxy) + + if infiltername and infiltername not in info["import_filters"]: + existing = "\n".join(sorted(info["import_filters"])) + logger.critical( + f"Unknown import filter: {infiltername}. Available filters:\n{existing}" + ) + raise RuntimeError("Invalid parameter") + + if filtername and filtername not in info["export_filters"]: + existing = "\n".join(sorted(info["export_filters"])) + logger.critical( + f"Unknown export filter: {filtername}. Available filters:\n{existing}" + ) + raise RuntimeError("Invalid parameter") + + logger.info("Converting.") result = proxy.convert( inpath, indata, @@ -108,11 +151,15 @@ def convert( if result is not None: # We got the file back over xmlrpc: if outpath: + logger.info(f"Writing to {outpath}.") with open(outpath, "wb") as outfile: outfile.write(result.data) else: # Return the result as a blob + logger.info(f"Returning {len(result.data)} bytes.") return result.data + else: + logger.info(f"Saved to {outpath}.") def compare( self, @@ -175,6 +222,11 @@ def compare( newpath = os.path.abspath(newpath) with ServerProxy(f"http://{self.server}:{self.port}", allow_none=True) as proxy: + logger.info("Connecting.") + logger.debug(f"Host: {self.server} Port: {self.port}") + self._connect(proxy) + + logger.info("Comparing.") result = proxy.compare( oldpath, olddata, @@ -186,16 +238,19 @@ def compare( if result is not None: # We got the file back over xmlrpc: if outpath: + logger.info(f"Writing to {outpath}.") with open(outpath, "wb") as outfile: outfile.write(result.data) else: # Return the result as a blob + logger.info(f"Returning {len(result.data)} bytes.") return result.data + else: + logger.info(f"Saved to {outpath}.") def converter_main(): logging.basicConfig() - logger.setLevel(logging.DEBUG) parser = argparse.ArgumentParser("unoconvert") parser.add_argument( @@ -259,7 +314,29 @@ def converter_main(): "Default is auto, and it will send the file as a path if the host is 127.0.0.1 or " "localhost, and binary data for other hosts.", ) + parser.add_argument( + "--verbose", + action="store_true", + dest="verbose", + help="Increase informational output to stderr.", + ) + parser.add_argument( + "--quiet", + action="store_true", + dest="quiet", + help="Decrease informational output to stderr.", + ) args = parser.parse_args() + + if args.verbose: + logger.setLevel(logging.DEBUG) + elif args.quiet: + logger.setLevel(logging.CRITICAL) + else: + logger.setLevel(logging.INFO) + if args.verbose and args.quiet: + logger.debug("Make up your mind, yo!") + client = UnoClient(args.host, args.port, args.host_location) if args.outfile == "-": @@ -291,7 +368,7 @@ def converter_main(): def comparer_main(): logging.basicConfig() - logger.setLevel(logging.DEBUG) + logger.setLevel(logging.INFO) parser = argparse.ArgumentParser("unocompare") parser.add_argument( @@ -331,8 +408,29 @@ def comparer_main(): "Default is auto, and it will send the file as a path if the host is 127.0.0.1 or " "localhost, and binary data for other hosts.", ) + parser.add_argument( + "--verbose", + action="store_true", + dest="verbose", + help="Increase informational output to stderr.", + ) + parser.add_argument( + "--quiet", + action="store_true", + dest="quiet", + help="Decrease informational output to stderr.", + ) args = parser.parse_args() + if args.verbose: + logger.setLevel(logging.DEBUG) + elif args.quiet: + logger.setLevel(logging.CRITICAL) + else: + logger.setLevel(logging.INFO) + if args.verbose and args.quiet: + logger.debug("Make up your mind, yo!") + client = UnoClient(args.host, args.port, args.host_location) if args.outfile == "-": diff --git a/src/unoserver/server.py b/src/unoserver/server.py index 23e29b7..34a2468 100644 --- a/src/unoserver/server.py +++ b/src/unoserver/server.py @@ -27,11 +27,13 @@ def __init__( addr_info = socket.getaddrinfo(addr[0], addr[1], proto=socket.IPPROTO_TCP) if len(addr_info) == 0: - raise RuntimeError(f"Could not get interface information for {addr[0]}:{addr[1]}") + raise RuntimeError( + f"Could not get interface information for {addr[0]}:{addr[1]}" + ) self.address_family = addr_info[0][0] self.socket_type = addr_info[0][1] - super().__init__(addr=addr_info[0][4], allow_none=allow_none) + super().__init__(addr=addr_info[0][4], allow_none=allow_none) class UnoServer: @@ -53,6 +55,8 @@ def __init__( self.xmlrcp_server = None def start(self, executable="libreoffice"): + import time + logger.info(f"Starting unoserver {__version__}.") connection = ( @@ -98,18 +102,35 @@ def signal_handler(signum, frame): if platform.system() != "Windows": signal.signal(signal.SIGHUP, signal_handler) + time.sleep(10) + self.xmlrcp_thread.start() return self.libreoffice_process def serve(self): # Create server - with XMLRPCServer( - (self.interface, int(self.port)), allow_none=True - ) as server: + with XMLRPCServer((self.interface, int(self.port)), allow_none=True) as server: self.xmlrcp_server = server server.register_introspection_functions() + @server.register_function + def info(): + conv = converter.UnoConverter( + interface=self.uno_interface, port=self.uno_port + ) + import_filters = conv.get_filter_names( + conv.get_available_import_filters() + ) + export_filters = conv.get_filter_names( + conv.get_available_export_filters() + ) + return { + "unoserver": __version__, + "import_filters": import_filters, + "export_filters": export_filters, + } + @server.register_function def convert( inpath=None, diff --git a/tests/test_integration.py b/tests/test_integration.py index 0cc4b49..9c68f7f 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -147,7 +147,7 @@ def test_explicit_export_filter(server_fixture, filename): @pytest.mark.parametrize("filename", ["simple.odt", "simple.xlsx"]) def test_invalid_explicit_export_filter_prints_available_filters( - server_fixture, filename + caplog, server_fixture, filename ): infile = os.path.join(TEST_DOCS, filename) @@ -156,10 +156,13 @@ def test_invalid_explicit_export_filter_prints_available_filters( sys.argv = ["unoconverter", "--filter", "asdasdasd", infile, outfile.name] try: client.converter_main() - except Fault as err: - assert "Office Open XML Text" in err.faultString - assert "writer8" in err.faultString - assert "writer_pdf_Export" in err.faultString + except RuntimeError: + errstr = caplog.text + print(errstr) + print("=" * 30) + assert "Office Open XML Text" in errstr + assert "writer8" in errstr + assert "writer_pdf_Export" in errstr def test_update_index(server_fixture): @@ -190,7 +193,7 @@ def test_update_index(server_fixture): def test_convert_not_local(): hostname = socket.gethostname() - cmd = ["unoserver", "--uno-port=2102", "--port=2103", f"--interface={hostname}"] + cmd = ["unoserver", "--uno-port=2104", "--port=2105", f"--interface={hostname}"] process = subprocess.Popen(cmd) try: # Wait for it to start @@ -205,7 +208,7 @@ def test_convert_not_local(): "unoconverter", "--host", hostname, - "--port=2103", + "--port=2105", infile, outfile.name, ] @@ -226,7 +229,7 @@ def test_convert_not_local(): def test_compare_not_local(): hostname = socket.gethostname() - cmd = ["unoserver", "--uno-port=2102", "--port=2103", f"--interface={hostname}"] + cmd = ["unoserver", "--uno-port=2104", "--port=2105", f"--interface={hostname}"] process = subprocess.Popen(cmd) try: # Wait for it to start @@ -242,7 +245,7 @@ def test_compare_not_local(): "unoconverter", "--host", hostname, - "--port=2103", + "--port=2105", infile1, infile2, outfile.name, diff --git a/tests/test_server.py b/tests/test_server.py index 37f87b9..d17ae25 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -27,6 +27,7 @@ def test_server_params(popen_mock, thread_mock): ] ) + @mock.patch("threading.Thread") @mock.patch("subprocess.Popen") def test_server_ipv6_params(popen_mock, thread_mock):