Named logger support and various server management enhancements by cpelley · Pull Request #132 · MetOffice/dagrunner

Carwyn Pelley (cpelley) · 2026-10-20T08:19:58Z

Attaching socket handler to a named logger by the client. This means being able to selectively choose what's captured by the DAGrunner logging server.
Ensure that logging server operates independently to clients. This means that the server now finishes and completes properly now and also doesn't wait for the client to cleanup it's handler.
Make logger name configurable via the DAGrunner global configuration file.
Updated tests accordingly.
AI helped a lot putting this new test file together.

Closes #125

… depends on clients closing their logging handlers

Rob Purvis (mo-robert-purvis) · 2026-10-21T07:28:09Z

+    finally:
+        if server_proc.poll() is None:
+            server_proc.kill()
+            server_proc.wait(timeout=5)


Looks weird waiting on a subprocess you've just killed, but I've read a bit around this, and this looks like it is to avoid the subprocess becoming a zombie.

Rob Purvis (mo-robert-purvis) · 2026-10-21T07:39:05Z

-    time.sleep(1)  # wait for db write to complete
    server_proc.terminate()
+    server_proc.wait(timeout=5)
+    assert server_proc.returncode == 0, "Server process did not exit normally"


This failed once when running pytest:

> assert server_proc.returncode == 0, "Server process did not exit normally" E AssertionError: Server process did not exit normally E assert -15 == 0 E + where -15 = <Popen: returncode: -15 args: ['/data/users/eppdev/conda/envs/scitools-2025_...>.returncode dagrunner/tests/utils/logging/test_integration.py:98: AssertionError

but a subsequent pytest run passed - weird.

Note that -15 means process was terminated with a SIGTERM

Maybe the assert should allow for 0 or -15 ?

Thanks Rob Purvis (@mo-robert-purvis), I'll investigate.

I think it may be that the 5s timeout was not enough, so the OS had to SIGTERM it.

Multiple servers are being started concurrently by different tests. One of those tests intentionally sends a SIGTERM to its server, but because the servers are not distinguishable from one another, the signal can inadvertently affect servers belonging to other tests. Those tests then fail unexpectedly because their server has been terminated rather than being cleanly shut down.

Rob Purvis (mo-robert-purvis) · 2026-10-21T08:00:07Z

    records and customise logging events.
    """

+    def _recv(self, size):


This is not covered by ptests

_recv is tested indirectly through integration tests, rather than by calling the method directly.

test_sqlitedb and test_named_logger in test_integration.py send records to the server, terminate it, and verify that the records were persisted. More specifically, test_process_termination in test_start_logging_server.py keeps a client connected while testing shutdown with no data, an incomplete header, and an incomplete record body. It checks that the server exits and saves earlier complete records. Those cases exercise the shutdown behaviour implemented by _recv.

Rob Purvis (mo-robert-purvis) · 2026-10-21T08:36:06Z

verbose is not used in the function.

Rob Purvis (mo-robert-purvis) · 2026-10-21T08:36:29Z

    - `sqlite_filepath`: The file path to the SQLite database.  Optional.
    - `host`: The host name of the server.  Optional.
    - `port`: The port number the server is listening on.  Optional.
    - `verbose`: Whether to print verbose output.  Optional.


verbose is not used in the function and should be removed.

Rob Purvis (mo-robert-purvis) · 2026-10-21T08:47:56Z

    # Logic around default values belong in the functions which use them.
    _INI_PARAMETERS = {
-        "dagrunner_logging": {"host": None, "port": None, "enabled": None},
+        "dagrunner_logging": {


ruff suggests these shouldn't be defined here

why? I'm not sure I agree in this particular case of a singleton which is instantiated within this same module. Either way not in scope here.

Ruff is run as part of pre-commit and is passed fine there:
https://github.com/MetOffice/dagrunner/actions/runs/39392634239/job/118527595887#step:9:1

Rob Purvis (mo-robert-purvis)

Some comments to consider

Carwyn Pelley (cpelley) · 2026-10-22T13:56:50Z

The non-zero error is I suspect caused by pytest running tests in parallel. Multiple tests start logging servers simultaneously, and those servers are not uniquely identifiable, leading to conflicts.

The fix is to configure the server port as 0, allowing the operating system to automatically allocate an available port. Each test can then retrieve the assigned port and connect its client to the corresponding logging server, ensuring that parallel test executions do not interfere with one another.

Carwyn Pelley (cpelley) · 2026-10-22T15:47:03Z

~~put back into draft. This handling of the server with the tests is annoying, especially since this problem is characteristic of parallel the tests and not real-life usage 😮‍💨~~
EDIT: Fixed -- I didn't push a local change. I have no pushed an amended commit and is ready to go.

Rob Purvis (mo-robert-purvis)

Looks good.

Carwyn Pelley (cpelley) added 6 commits October 19, 2026 17:18

Support for named loggers

583b7e9

add logger name to the configuration

6739548

using the specifieed logger

fe96832

TEST: fixed handler cleanup

3d14fc8

BUG: independently operated logging server. Server shutdown no longer…

87237fc

… depends on clients closing their logging handlers

TEST: Missing test

521fc68

Carwyn Pelley (cpelley) self-assigned this Oct 20, 2026

Carwyn Pelley (cpelley) added the enhancement New feature or request label Oct 20, 2026

Carwyn Pelley (cpelley) requested review from Anzer Khan (Anzerkhan27) and Rob Purvis (mo-robert-purvis) October 20, 2026 08:22

Rob Purvis (mo-robert-purvis) reviewed Oct 21, 2026

View reviewed changes

Carwyn Pelley (cpelley) requested a review from Rob Purvis (mo-robert-purvis) October 22, 2026 13:56

Carwyn Pelley (cpelley) marked this pull request as draft October 22, 2026 15:47

review changes, avoid overlapping logging servers in testing

6bb1f99

Carwyn Pelley (cpelley) force-pushed the logger_enhancecment branch 2 times, most recently from 474d545 to 6bb1f99 Compare October 22, 2026 15:50

Carwyn Pelley (cpelley) marked this pull request as ready for review October 22, 2026 15:52

Rob Purvis (mo-robert-purvis) approved these changes Oct 27, 2026

View reviewed changes

Carwyn Pelley (cpelley) merged commit 3fc9be3 into main Oct 27, 2026
6 checks passed

Carwyn Pelley (cpelley) deleted the logger_enhancecment branch October 27, 2026 12:42

Carwyn Pelley (cpelley) requested review from Jonathan Davies (jdjonathandavies) and removed request for Jonathan Davies (jdjonathandavies) November 6, 2026 14:11

Conversation

Carwyn Pelley (cpelley) commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rob Purvis (mo-robert-purvis) Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Carwyn Pelley (cpelley) Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Carwyn Pelley (cpelley) Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rob Purvis (mo-robert-purvis) left a comment

Choose a reason for hiding this comment

Uh oh!

Carwyn Pelley (cpelley) commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Carwyn Pelley (cpelley) commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rob Purvis (mo-robert-purvis) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Carwyn Pelley (cpelley) commented Oct 20, 2026 •

edited

Loading

Rob Purvis (mo-robert-purvis) Oct 21, 2026 •

edited

Loading

Carwyn Pelley (cpelley) Oct 22, 2026 •

edited

Loading

Carwyn Pelley (cpelley) Oct 22, 2026 •

edited

Loading

Carwyn Pelley (cpelley) commented Oct 22, 2026 •

edited

Loading

Carwyn Pelley (cpelley) commented Oct 22, 2026 •

edited

Loading