diff --git a/eventsourcingdb/container.py b/eventsourcingdb/container.py index 1729f61..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__) @@ -36,6 +37,9 @@ PING_TIMEOUT_SECONDS = 1 POLL_INTERVAL_SECONDS = 0.1 +V1_PING_PATH = "/api/v1/ping" +V2_PING_PATH = "/api/v2/system/ping" + def _format_seconds(seconds: float) -> str: # Whole seconds without a fraction, others as short as possible, such as 1.5. @@ -328,23 +332,44 @@ 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: + 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. 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 + if status not in STATUSES_WITHOUT_V2_API: + return False + + 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 + + # 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=timeout) except requests.RequestException: - 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 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..05c83ca 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,136 @@ 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(204, id='another success status'), + 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]}) + + # 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)) + 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] + 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: @staticmethod def test_is_30_seconds_by_default() -> None: @@ -230,7 +381,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 +391,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 +404,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(