From 83e583edf7a37218b9cfc00de472b310f886cfc3 Mon Sep 17 00:00:00 2001 From: Golo Roden Date: Fri, 23 Oct 2026 18:18:38 +0200 Subject: [PATCH 1/4] fix: Wait for the database in the test container through the v2 API, and through the v1 API only where the server lacks the v2 API. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CiuYmw4N6SSzfFyrwftUEj --- eventsourcingdb/container.py | 44 +++++++--- tests/test_container.py | 165 ++++++++++++++++++++++++++++++++--- 2 files changed, 188 insertions(+), 21 deletions(-) diff --git a/eventsourcingdb/container.py b/eventsourcingdb/container.py index 1729f61..dd34883 100644 --- a/eventsourcingdb/container.py +++ b/eventsourcingdb/container.py @@ -36,6 +36,15 @@ PING_TIMEOUT_SECONDS = 1 POLL_INTERVAL_SECONDS = 0.1 +V1_PING_PATH = "/api/v1/ping" +V2_PING_PATH = "/api/v2/system/ping" + +# A server without the v2 API answers its ping with one of these statuses. +# They are stated here rather than taken from the client, so that the +# readiness of the container that tests the client does not depend on the way +# the client chooses its API, which those tests are meant to check. +STATUSES_WITHOUT_V2_API = (HTTPStatus.NOT_FOUND, HTTPStatus.METHOD_NOT_ALLOWED) + def _format_seconds(seconds: float) -> str: # Whole seconds without a fraction, others as short as possible, such as 1.5. @@ -328,23 +337,38 @@ def _wait_until_reachable(self, container: DockerContainer, deadline: float) -> return None mapped_port = self._extract_port_from_container_info(container.attrs) - if mapped_port is not None and self._check_endpoint_available( - f"http://{self._host}:{mapped_port}/api/v1/ping", - min(remaining, PING_TIMEOUT_SECONDS), + if mapped_port is not None and self._is_database_ready( + f"http://{self._host}:{mapped_port}", deadline ): return mapped_port time.sleep(max(min(POLL_INTERVAL_SECONDS, deadline - time.monotonic()), 0)) - def _check_endpoint_available(self, url: str, timeout: float) -> bool: - try: - response = requests.get(url, timeout=timeout) - except requests.RequestException: + def _is_database_ready(self, base_url: str, deadline: float) -> bool: + # The v1 API is asked only where the server lacks the v2 API, since a + # later release of EventSourcingDB removes it. Any other failure means + # that the server is not ready yet, so the next attempt asks the v2 API + # again, rather than remembering an answer of a server that may still + # be starting. + status = self._ping(f"{base_url}{V2_PING_PATH}", deadline) + if status == HTTPStatus.OK: + return True + if status not in STATUSES_WITHOUT_V2_API: return False - return self._check_response_ok(response) - def _check_response_ok(self, response: requests.Response) -> bool: - return response is not None and response.status_code == HTTPStatus.OK + return self._ping(f"{base_url}{V1_PING_PATH}", deadline) == HTTPStatus.OK + + def _ping(self, url: str, deadline: float) -> int | None: + remaining = deadline - time.monotonic() + if remaining <= 0: + return None + + # The ping needs no API token, so it gets none. + try: + response = requests.get(url, timeout=min(remaining, PING_TIMEOUT_SECONDS)) + except requests.RequestException: + return None + return response.status_code def with_api_token(self, token: str) -> "Container": self._api_token = token diff --git a/tests/test_container.py b/tests/test_container.py index 37c039c..61c65ba 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -12,7 +12,11 @@ from docker.models.images import ImageCollection from eventsourcingdb import Container -from eventsourcingdb.container import DEFAULT_STARTUP_TIMEOUT_SECONDS, START_LABEL +from eventsourcingdb.container import ( + DEFAULT_STARTUP_TIMEOUT_SECONDS, + PING_TIMEOUT_SECONDS, + START_LABEL, +) from .shared.image_tag import get_image_tag @@ -21,6 +25,9 @@ FAKE_PORT = 49153 POLL_GUARD_SECONDS = 2 +V1_PING_URL = f'http://localhost:{FAKE_PORT}/api/v1/ping' +V2_PING_URL = f'http://localhost:{FAKE_PORT}/api/v2/system/ping' + class CreatedContainers: """ @@ -116,17 +123,31 @@ def reload(self) -> None: class FakePing: """ - Answers the pings with the given statuses in turn, and with the last one - from then on. + Answers the pings of each URL with the given answers in turn, and with the + last one from then on. An answer is a status, or an error that the ping + raises instead, such as when the connection is refused or the ping hits its + bound. A URL without answers is answered with 404, as a server answers a + path that it does not know. """ - def __init__(self, statuses: list[int]) -> None: - self.statuses = statuses + def __init__(self, answers: dict[str, list[int | requests.RequestException]]) -> None: + self.answers = answers self.urls: list[str] = [] + self.timeouts: list[float | None] = [] - def __call__(self, url: str, timeout: float) -> SimpleNamespace: + def __call__(self, url: str, timeout: float | None = None) -> SimpleNamespace: self.urls.append(url) - return SimpleNamespace(status_code=self.statuses[min(len(self.urls), len(self.statuses)) - 1]) + self.timeouts.append(timeout) + + # As requests does, which refuses a bound that leaves no time at all. + if timeout is not None and timeout <= 0: + raise ValueError(f'The ping must have time left, but its bound is {timeout}.') + + answers = self.answers.get(url, [404]) + answer = answers[min(self.urls.count(url), len(answers)) - 1] + if isinstance(answer, requests.RequestException): + raise answer + return SimpleNamespace(status_code=answer) def listen_on_another_port(command: list[str]) -> list[str]: @@ -205,6 +226,128 @@ async def test_leaves_containers_alone_that_it_did_not_start() -> None: other_container.remove(force=True) +class TestWaitingForTheDatabase: + """ + Every attempt asks the ping of the v2 API, and the ping of the v1 API only + if the server answers that with 404 or 405, as a server without the v2 API + does, so that the container becomes ready against servers with either API. + """ + + @staticmethod + def test_is_ready_once_the_v2_api_answers_its_ping_without_asking_the_v1_api( + monkeypatch: pytest.MonkeyPatch, + ) -> None: + ping = FakePing({V2_PING_URL: [200], V1_PING_URL: [200]}) + monkeypatch.setattr(requests, 'get', ping) + + mapped_port = Container()._wait_until_reachable( + FakeContainer(published_from_poll=1), time.monotonic() + POLL_GUARD_SECONDS + ) + + assert mapped_port == FAKE_PORT + assert ping.urls == [V2_PING_URL] + + @staticmethod + def test_is_ready_once_the_v2_api_answers_its_ping_if_the_server_lacks_the_v1_api( + monkeypatch: pytest.MonkeyPatch, + ) -> None: + ping = FakePing({V2_PING_URL: [200], V1_PING_URL: [404]}) + monkeypatch.setattr(requests, 'get', ping) + + mapped_port = Container()._wait_until_reachable( + FakeContainer(published_from_poll=1), time.monotonic() + POLL_GUARD_SECONDS + ) + + assert mapped_port == FAKE_PORT + assert ping.urls == [V2_PING_URL] + + @staticmethod + @pytest.mark.parametrize('status', [404, 405]) + def test_asks_the_v1_api_within_the_same_attempt_if_the_server_lacks_the_v2_api( + status: int, monkeypatch: pytest.MonkeyPatch + ) -> None: + ping = FakePing({V2_PING_URL: [status], V1_PING_URL: [200]}) + monkeypatch.setattr(requests, 'get', ping) + fake_container = FakeContainer(published_from_poll=1) + + mapped_port = Container()._wait_until_reachable( + fake_container, time.monotonic() + POLL_GUARD_SECONDS + ) + + assert mapped_port == FAKE_PORT + assert ping.urls == [V2_PING_URL, V1_PING_URL] + assert fake_container.polls == 1 + assert all( + timeout is not None and 0 < timeout <= PING_TIMEOUT_SECONDS + for timeout in ping.timeouts + ), f'Every ping must be bounded, but the bounds were {ping.timeouts}.' + + @staticmethod + @pytest.mark.parametrize( + 'failure', + [ + pytest.param(503, id='another status'), + pytest.param(requests.exceptions.ReadTimeout('Read timed out.'), id='bound per request'), + pytest.param(requests.exceptions.ConnectionError('Connection refused.'), id='refused'), + ], + ) + def test_tries_again_without_asking_the_v1_api_if_the_ping_of_the_v2_api_fails_otherwise( + failure: int | requests.RequestException, monkeypatch: pytest.MonkeyPatch + ) -> None: + ping = FakePing({V2_PING_URL: [failure, 200], V1_PING_URL: [200]}) + monkeypatch.setattr(requests, 'get', ping) + fake_container = FakeContainer(published_from_poll=1) + + mapped_port = Container()._wait_until_reachable( + fake_container, time.monotonic() + POLL_GUARD_SECONDS + ) + + assert mapped_port == FAKE_PORT + assert ping.urls == [V2_PING_URL, V2_PING_URL] + assert fake_container.polls == 2 + + @staticmethod + @pytest.mark.parametrize( + 'failure', + [ + pytest.param(503, id='another status'), + pytest.param(requests.exceptions.ReadTimeout('Read timed out.'), id='bound per request'), + ], + ) + def test_starts_the_next_attempt_with_the_v2_api_if_the_ping_of_the_v1_api_fails( + failure: int | requests.RequestException, monkeypatch: pytest.MonkeyPatch + ) -> None: + ping = FakePing({V2_PING_URL: [404, 200], V1_PING_URL: [failure]}) + monkeypatch.setattr(requests, 'get', ping) + fake_container = FakeContainer(published_from_poll=1) + + mapped_port = Container()._wait_until_reachable( + fake_container, time.monotonic() + POLL_GUARD_SECONDS + ) + + assert mapped_port == FAKE_PORT + assert ping.urls == [V2_PING_URL, V1_PING_URL, V2_PING_URL] + assert fake_container.polls == 2 + + @staticmethod + def test_does_not_ask_the_v1_api_once_the_ping_of_the_v2_api_used_up_the_time( + monkeypatch: pytest.MonkeyPatch, + ) -> None: + ping = FakePing({V2_PING_URL: [404], V1_PING_URL: [200]}) + deadline = time.monotonic() + 0.5 + + def answer_once_the_time_is_up(url: str, timeout: float | None = None) -> SimpleNamespace: + time.sleep(max(deadline - time.monotonic(), 0)) + return ping(url, timeout) + + monkeypatch.setattr(requests, 'get', answer_once_the_time_is_up) + + mapped_port = Container()._wait_until_reachable(FakeContainer(published_from_poll=1), deadline) + + assert mapped_port is None + assert ping.urls == [V2_PING_URL] + + class TestStartupTimeout: @staticmethod def test_is_30_seconds_by_default() -> None: @@ -230,7 +373,7 @@ def test_rejects_a_startup_timeout_that_is_not_a_finite_number_greater_than_zero @staticmethod def test_reads_the_port_again_on_every_poll(monkeypatch: pytest.MonkeyPatch) -> None: - ping = FakePing([200]) + ping = FakePing({V2_PING_URL: [200]}) monkeypatch.setattr(requests, 'get', ping) fake_container = FakeContainer(published_from_poll=3) @@ -240,11 +383,11 @@ def test_reads_the_port_again_on_every_poll(monkeypatch: pytest.MonkeyPatch) -> assert mapped_port == FAKE_PORT assert fake_container.polls == 3 - assert ping.urls == [f'http://localhost:{FAKE_PORT}/api/v1/ping'] + assert ping.urls == [V2_PING_URL] @staticmethod def test_accepts_only_status_200_from_the_ping(monkeypatch: pytest.MonkeyPatch) -> None: - ping = FakePing([204, 500, 200]) + ping = FakePing({V2_PING_URL: [204, 500, 200]}) monkeypatch.setattr(requests, 'get', ping) fake_container = FakeContainer(published_from_poll=1) @@ -253,7 +396,7 @@ def test_accepts_only_status_200_from_the_ping(monkeypatch: pytest.MonkeyPatch) ) assert mapped_port == FAKE_PORT - assert len(ping.urls) == 3 + assert ping.urls == [V2_PING_URL] * 3 @staticmethod def test_replaces_a_running_container_that_does_not_become_reachable_in_time( From c230038f9497954c7ffb9e74db66e93229efa1b8 Mon Sep 17 00:00:00 2001 From: Golo Roden Date: Fri, 23 Oct 2026 18:32:01 +0200 Subject: [PATCH 2/4] chore: Pass the bound of the readiness ping by name, so that bandit sees that the request has one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CiuYmw4N6SSzfFyrwftUEj --- eventsourcingdb/container.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eventsourcingdb/container.py b/eventsourcingdb/container.py index dd34883..e8ae363 100644 --- a/eventsourcingdb/container.py +++ b/eventsourcingdb/container.py @@ -363,9 +363,12 @@ def _ping(self, url: str, deadline: float) -> int | None: if remaining <= 0: return None + # Bandit sees that a request has a bound only if it is passed by name. + timeout = min(remaining, PING_TIMEOUT_SECONDS) + # The ping needs no API token, so it gets none. try: - response = requests.get(url, timeout=min(remaining, PING_TIMEOUT_SECONDS)) + response = requests.get(url, timeout=timeout) except requests.RequestException: return None return response.status_code From 07c8a0d841b31cde447d64c03265de23e56e04e9 Mon Sep 17 00:00:00 2001 From: Golo Roden Date: Fri, 23 Oct 2026 18:32:28 +0200 Subject: [PATCH 3/4] chore: Tell a server without the v2 API in the wait of the test container by the statuses that the client tells it by. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CiuYmw4N6SSzfFyrwftUEj --- eventsourcingdb/container.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/eventsourcingdb/container.py b/eventsourcingdb/container.py index e8ae363..8d3f1d8 100644 --- a/eventsourcingdb/container.py +++ b/eventsourcingdb/container.py @@ -16,6 +16,7 @@ from docker.models.containers import Container as DockerContainer from .client import Client +from .transport.statuses_without_v2_api import STATUSES_WITHOUT_V2_API logger = logging.getLogger(__name__) @@ -39,12 +40,6 @@ V1_PING_PATH = "/api/v1/ping" V2_PING_PATH = "/api/v2/system/ping" -# A server without the v2 API answers its ping with one of these statuses. -# They are stated here rather than taken from the client, so that the -# readiness of the container that tests the client does not depend on the way -# the client chooses its API, which those tests are meant to check. -STATUSES_WITHOUT_V2_API = (HTTPStatus.NOT_FOUND, HTTPStatus.METHOD_NOT_ALLOWED) - def _format_seconds(seconds: float) -> str: # Whole seconds without a fraction, others as short as possible, such as 1.5. @@ -349,7 +344,10 @@ def _is_database_ready(self, base_url: str, deadline: float) -> bool: # later release of EventSourcingDB removes it. Any other failure means # that the server is not ready yet, so the next attempt asks the v2 API # again, rather than remembering an answer of a server that may still - # be starting. + # be starting. The statuses that tell a server without the v2 API are + # shared with the client, but not the way the client chooses its API, + # so that the readiness of the container that tests the client does + # not depend on what those tests are meant to check. status = self._ping(f"{base_url}{V2_PING_PATH}", deadline) if status == HTTPStatus.OK: return True From a2638351af63b97137c469a521f1220157526e0e Mon Sep 17 00:00:00 2001 From: Golo Roden Date: Fri, 23 Oct 2026 18:51:37 +0200 Subject: [PATCH 4/4] chore: Test that the wait of the test container refuses a success other than 200 from the ping of the v1 API, and bounds a ping by the time left. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CiuYmw4N6SSzfFyrwftUEj --- tests/test_container.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_container.py b/tests/test_container.py index 61c65ba..05c83ca 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -310,6 +310,7 @@ def test_tries_again_without_asking_the_v1_api_if_the_ping_of_the_v2_api_fails_o @pytest.mark.parametrize( 'failure', [ + pytest.param(204, id='another success status'), pytest.param(503, id='another status'), pytest.param(requests.exceptions.ReadTimeout('Read timed out.'), id='bound per request'), ], @@ -334,7 +335,11 @@ def test_does_not_ask_the_v1_api_once_the_ping_of_the_v2_api_used_up_the_time( monkeypatch: pytest.MonkeyPatch, ) -> None: ping = FakePing({V2_PING_URL: [404], V1_PING_URL: [200]}) - deadline = time.monotonic() + 0.5 + + # Less time than a single ping may take, so that the time left bounds + # the ping rather than the bound of every ping. + time_left = PING_TIMEOUT_SECONDS / 2 + deadline = time.monotonic() + time_left def answer_once_the_time_is_up(url: str, timeout: float | None = None) -> SimpleNamespace: time.sleep(max(deadline - time.monotonic(), 0)) @@ -346,6 +351,9 @@ def answer_once_the_time_is_up(url: str, timeout: float | None = None) -> Simple assert mapped_port is None assert ping.urls == [V2_PING_URL] + assert ping.timeouts[0] is not None and 0 < ping.timeouts[0] <= time_left, ( + f'The ping must be bounded by the time left, {time_left}s, but was bounded by {ping.timeouts[0]}.' + ) class TestStartupTimeout: