From 242a38ff8721c928196bd0a1e57b9d5666b184b3 Mon Sep 17 00:00:00 2001 From: Christopher Lupp Date: Sun, 30 Aug 2026 21:06:36 -0400 Subject: [PATCH] Derive DisciplineServer from the servicer base class (#70) DisciplineServer inherited disc.DisciplineService, the generated experimental static-call API, rather than DisciplineServiceServicer, which is what add_DisciplineServiceServicer_to_server is written against. Nothing broke in practice -- the registration helper looks handlers up by name and DisciplineServer defines all eight RPCs itself -- but the class did not inherit the UNIMPLEMENTED defaults a servicer base provides, so a subclass omitting an RPC would resolve to a static network-call stub instead of returning a clean UNIMPLEMENTED status. ExplicitServer and ImplicitServer already mixed in the correct servicers; this brings the base class in line. --- CHANGELOG.md | 14 +++++++++++++ philote_mdo/general/discipline_server.py | 2 +- tests/test_discipline_server.py | 26 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5e58d..e94db42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes +- `DisciplineServer` inherited `DisciplineService`, the generated experimental + static-call API, rather than `DisciplineServiceServicer`, the servicer base + that `add_DisciplineServiceServicer_to_server` is written against. No + shipping code path reached the difference, because `DisciplineServer` + overrides all eight RPCs itself, but the class did not inherit the + `UNIMPLEMENTED` defaults a servicer base provides. An RPC left undefined -- + for instance one added to the `.proto` and not yet implemented -- resolved + instead to a static client stub, which registers without complaint and then + fails at call time by treating the `ServicerContext` as a target address, + raising out of the handler without setting a status, so the caller saw + `UNKNOWN` rather than `UNIMPLEMENTED`. The base is now + `DisciplineServiceServicer`, matching `ExplicitServer` and `ImplicitServer` + (#70). + - Fixed `RemoteExplicitComponent` and `RemoteImplicitComponent` sending the raw constructor keyword arguments to the server rather than the component's resolved options. An option that reached the component by any other route -- diff --git a/philote_mdo/general/discipline_server.py b/philote_mdo/general/discipline_server.py index 9decdfa..1dff413 100644 --- a/philote_mdo/general/discipline_server.py +++ b/philote_mdo/general/discipline_server.py @@ -46,7 +46,7 @@ from philote_mdo.utils.validation import PhiloteValidationError, validate_shape -class DisciplineServer(disc.DisciplineService): +class DisciplineServer(disc.DisciplineServiceServicer): """ Base class for all server classes. """ diff --git a/tests/test_discipline_server.py b/tests/test_discipline_server.py index 7a357d8..5c49ae3 100644 --- a/tests/test_discipline_server.py +++ b/tests/test_discipline_server.py @@ -38,6 +38,7 @@ from philote_mdo.general import Discipline, DisciplineServer from philote_mdo.utils.validation import PhiloteValidationError import philote_mdo.generated.data_pb2 as data +import philote_mdo.generated.disciplines_pb2_grpc as disc class TestDisciplineServer(unittest.TestCase): @@ -610,6 +611,31 @@ def test_setup_general_exception_aborts(self): self.assertEqual(args[0][0], grpc.StatusCode.INTERNAL) self.assertIn("Setup failed", args[0][1]) + def test_servicer_base_class(self): + """ + Tests that the server derives from the servicer, not the static API. + """ + self.assertTrue(issubclass(DisciplineServer, disc.DisciplineServiceServicer)) + self.assertNotIn(disc.DisciplineService, DisciplineServer.__mro__) + + def test_missing_rpc_falls_back_to_servicer_default(self): + """ + Tests that an RPC a subclass fails to define resolves to the servicer's + UNIMPLEMENTED default rather than to the static-call API. + """ + skip = ("__dict__", "__weakref__", "GetInfo") + attrs = {k: v for k, v in vars(DisciplineServer).items() if k not in skip} + partial_cls = type("PartialServer", DisciplineServer.__bases__, attrs) + + # the inherited GetInfo must come from the servicer, not the static API + self.assertIs(partial_cls.GetInfo, disc.DisciplineServiceServicer.GetInfo) + + context = Mock() + with self.assertRaises(NotImplementedError): + partial_cls().GetInfo(Empty(), context) + + context.set_code.assert_called_once_with(grpc.StatusCode.UNIMPLEMENTED) + if __name__ == "__main__": unittest.main(verbosity=2)