From 4c38473865d3528c903e4ab7cdf8b0007748edc3 Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Thu, 29 Aug 2024 08:34:06 +0200 Subject: [PATCH] server-unified: generalize for serial/parallel approach that is not django-specific --- Dockerfile | 2 +- bugsink/scripts/server_unified.py | 56 +++++++++++++++++++----------- bugsink/tests.py | 57 +++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 21 deletions(-) diff --git a/Dockerfile b/Dockerfile index 6541d57..255f143 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,4 +32,4 @@ RUN ["bugsink-manage", "migrate", "snappea", "--database=snappea"] EXPOSE 8000 -CMD [ "bugsink-server-unified", "gunicorn", "--bind=0.0.0.0:8000", "--workers=10", "--access-logfile", "-", "bugsink.wsgi", "UNIFIED_WITH", "bugsink-runsnappea"] +CMD [ "bugsink-server-unified", "bugsink-manage", "check", "--deploy", "--fail-level", "WARNING", "AMP_AMP", "gunicorn", "--bind=0.0.0.0:8000", "--workers=10", "--access-logfile", "-", "bugsink.wsgi", "UNIFIED_WITH", "bugsink-runsnappea"] diff --git a/bugsink/scripts/server_unified.py b/bugsink/scripts/server_unified.py index ea756e6..fe58640 100644 --- a/bugsink/scripts/server_unified.py +++ b/bugsink/scripts/server_unified.py @@ -39,25 +39,20 @@ class ParentProcess: child.wait() def pre_start(self): - # I'd rather pull this out of server_unified.py, but I don't know how to do that in a way that works with - # Docker: The recommended way of running CMD in a Dockerfile is to use the exec form, which doesn't allow for - # running a script that does some setup before starting the main process, i.e. doesn't allow for '&&'). - # Recommended here means: warning about signal-handling if you choose the other form. - # - # I also don't want to introduce further arg-parsing (distinguishing between serial and parallel start) so here - # we have it. - if sys.argv[1:2] == ["NO_DEPLOY_CHECK"]: - check = subprocess.run(["bugsink-manage", "check", "--fail-level", "WARNING"]) - else: - check = subprocess.run(["bugsink-manage", "check", "--deploy", "--fail-level", "WARNING"]) - if check.returncode != 0: - # print("Server-unified failed to start because 'bugsink-manage check' failed.") superfluous - sys.exit(1) + # I'd rather this was not needed, I don't know how to do that in a way that works with Docker: The recommended + # way of running CMD in a Dockerfile is to use the exec form, which doesn't allow for running a script that does + # some setup before starting the main process, i.e. doesn't allow for '&&'. Recommended here means: warning + # about signal-handling if you choose the other form. + + for args in self.get_pre_start_command_args(sys.argv): + print("Server-unified 'pre-start' process:", " ".join(args)) + proc = subprocess.run(args) + if proc.returncode != 0: + sys.exit(proc.returncode) def start_children(self): - # Start the server # Leaving stdout and stderr as None will make the output of the child processes be passed as our own. - for args in self.parse_args(): + for args in self.get_parallel_command_args(sys.argv): try: child = subprocess.Popen(args) except Exception: @@ -86,14 +81,35 @@ class ParentProcess: children_are_alive = False self.terminate_children(except_child=child) - def parse_args(self): + @classmethod + def get_pre_start_command_args(self, argv): + """Splits our own arguments into a list of args for each of the pre-start commands, we split on "AMP_AMP".""" + + # We don't want to pass the first argument, as that is the script name + args = argv[1:] + + result = [] + this = [] + for arg in args: + if arg == "AMP_AMP": + # AMP_AMP serves as a terminator here, i.e. we only add-to-result when we encounter it, the last bit + # is never addeded (it will be dealt with as the set of parallel commands) + result.append(this) + this = [] + else: + this.append(arg) + + return result + + @classmethod + def get_parallel_command_args(self, argv): """Splits our own arguments into a list of args for each of the children each, we split on "UNIFIED_WITH".""" # We don't want to pass the first argument, as that is the script name - args = sys.argv[1:] + args = argv[1:] - if args[:1] == ["NO_DEPLOY_CHECK"]: - args = args[1:] + while "AMP_AMP" in args: + args = args[args.index("AMP_AMP") + 1:] result = [[]] for arg in args: diff --git a/bugsink/tests.py b/bugsink/tests.py index 598ad32..24a3281 100644 --- a/bugsink/tests.py +++ b/bugsink/tests.py @@ -7,6 +7,7 @@ from .volume_based_condition import VolumeBasedCondition from .streams import ( compress_with_zlib, GeneratorReader, WBITS_PARAM_FOR_GZIP, WBITS_PARAM_FOR_DEFLATE, MaxDataReader, MaxDataWriter, zlib_generator, brotli_generator) +from .scripts.server_unified import ParentProcess def apply_n(f, n, v): @@ -129,3 +130,59 @@ class StreamsTestCase(RegularTestCase): with self.assertRaises(ValueError): writer.write(b"hellohello") + + +class ServerUnifiedTestCase(RegularTestCase): + + def test_arg_parsing(self): + def _check(argv, expected_pre_start, expected_parallel): + pre_start = ParentProcess.get_pre_start_command_args(argv) + parallel = ParentProcess.get_parallel_command_args(argv) + self.assertEqual(expected_pre_start, pre_start) + self.assertEqual(expected_parallel, parallel) + + _check( + # meaning: a single empty command (which would lead to a failure). It's the meaningless case anyway, so I'm + # not going to make any special-case handling for it. In other words: there must be at least one command + # (and even that is quite meaningless, since you could just run the command directly). + ["script.py"], + [], + [[]], + ) + + _check( + ["script.py", "a", "b"], + [], + [["a", "b"]], + ) + + _check( + ["script.py", "a", "b", "UNIFIED_WITH", "c", "d", "UNIFIED_WITH", "e", "f"], + [], + [["a", "b"], ["c", "d"], ["e", "f"]], + ) + + _check( + ["script.py", "a", "b", "AMP_AMP", "c", "d", "UNIFIED_WITH", "e", "f"], + [["a", "b"]], + [["c", "d"], ["e", "f"]], + ) + + _check( + ["script.py", "a", "b", "UNIFIED_WITH", "c", "d", "UNIFIED_WITH", "e", "f"], + [], + [["a", "b"], ["c", "d"], ["e", "f"]], + ) + + _check( + ["script.py", "a", "b", "AMP_AMP", "c", "d", "AMP_AMP", "e", "f"], + [["a", "b"], ["c", "d"]], + [["e", "f"]], + ) + + _check( + ["script.py", "a", "b", "AMP_AMP", "c", "d", "AMP_AMP", + "e", "f", "UNIFIED_WITH", "g", "h", "UNIFIED_WITH", "i", "j"], + [["a", "b"], ["c", "d"]], + [["e", "f"], ["g", "h"], ["i", "j"]], + )