From 5c53eaaef5778921fbffd7fb468871634f3867f7 Mon Sep 17 00:00:00 2001
From: Anton Agestam <anton.agestam@aiven.io>
Date: Wed, 31 May 2023 18:21:37 +0200
Subject: [PATCH] chore: Add partial stubs for the isodate library

This revealed a surprising bug in `PollTimeout`, where we don't handle
`parse_duration()` returning `isodate.duration.Duration`.
---
 karapace/backup/poll_timeout.py        | 14 ++++++++++----
 mypy.ini                               |  4 +---
 stubs/isodate/__init__.pyi             |  3 +++
 stubs/isodate/duration.pyi             | 21 +++++++++++++++++++++
 stubs/isodate/isoduration.pyi          | 18 ++++++++++++++++++
 stubs/isodate/py.typed                 |  0
 tests/unit/backup/test_poll_timeout.py | 13 +++++++++++--
 7 files changed, 64 insertions(+), 9 deletions(-)
 create mode 100644 stubs/isodate/__init__.pyi
 create mode 100644 stubs/isodate/duration.pyi
 create mode 100644 stubs/isodate/isoduration.pyi
 create mode 100644 stubs/isodate/py.typed

diff --git a/karapace/backup/poll_timeout.py b/karapace/backup/poll_timeout.py
index 09df95bf1..0a1b9e157 100644
--- a/karapace/backup/poll_timeout.py
+++ b/karapace/backup/poll_timeout.py
@@ -7,8 +7,9 @@
 from datetime import timedelta
 from functools import cached_property
 from isodate import duration_isoformat, parse_duration
+from typing import Final
 
-__all__ = ["PollTimeout"]
+__all__ = ("PollTimeout",)
 
 
 class PollTimeout:
@@ -19,9 +20,14 @@ class PollTimeout:
     """
 
     def __init__(self, value: str | timedelta) -> None:
-        self.__value = value if isinstance(value, timedelta) else parse_duration(value)
-        if self.__value // timedelta(seconds=1) < 1:
-            raise ValueError(f"Poll timeout MUST be at least one second, got: {self}")
+        duration = value if isinstance(value, timedelta) else parse_duration(value)
+        # parse_duration() returns Duration objects for values that cannot be
+        # represented by a datetime.timedelta.
+        if not isinstance(duration, timedelta):
+            raise ValueError(f"Poll timeout must be less than one year, got: {duration}")
+        if isinstance(duration, timedelta) and duration < timedelta(seconds=1):
+            raise ValueError(f"Poll timeout must be at least one second, got: {duration}")
+        self.__value: Final = duration
 
     @classmethod
     def default(cls) -> PollTimeout:
diff --git a/mypy.ini b/mypy.ini
index f5d48f9f1..861d70628 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -1,4 +1,5 @@
 [mypy]
+mypy_path = $MYPY_CONFIG_FILE_DIR/stubs
 python_version = 3.8
 packages = karapace
 show_error_codes = True
@@ -118,8 +119,5 @@ ignore_errors = True
 [mypy-kafka.*]
 ignore_missing_imports = True
 
-[mypy-isodate.*]
-ignore_missing_imports = True
-
 [mypy-networkx.*]
 ignore_missing_imports = True
diff --git a/stubs/isodate/__init__.pyi b/stubs/isodate/__init__.pyi
new file mode 100644
index 000000000..eb1aefe7d
--- /dev/null
+++ b/stubs/isodate/__init__.pyi
@@ -0,0 +1,3 @@
+from isodate.isoduration import duration_isoformat, parse_duration
+
+__all__ = ("duration_isoformat", "parse_duration")
diff --git a/stubs/isodate/duration.pyi b/stubs/isodate/duration.pyi
new file mode 100644
index 000000000..46f93d801
--- /dev/null
+++ b/stubs/isodate/duration.pyi
@@ -0,0 +1,21 @@
+from decimal import Decimal
+
+import datetime
+
+class Duration:
+    months: Decimal
+    years: Decimal
+    tdelta: datetime.timedelta
+
+    def __init__(
+        self,
+        days: int = ...,
+        seconds: int = ...,
+        microseconds: int = ...,
+        milliseconds: int = ...,
+        minutes: int = ...,
+        hours: int = ...,
+        weeks: int = ...,
+        months: int = ...,
+        years: int = ...,
+    ) -> None: ...
diff --git a/stubs/isodate/isoduration.pyi b/stubs/isodate/isoduration.pyi
new file mode 100644
index 000000000..1652fdd80
--- /dev/null
+++ b/stubs/isodate/isoduration.pyi
@@ -0,0 +1,18 @@
+from .duration import Duration
+from typing import Literal, overload
+
+import datetime
+
+@overload
+def parse_duration(
+    datestring: str,
+) -> datetime.timedelta | Duration: ...
+@overload
+def parse_duration(
+    datestring: str,
+    as_timedelta_if_possible: Literal[False],
+) -> Duration: ...
+def duration_isoformat(
+    tduration: Duration | datetime.timedelta,
+    format: str = ...,
+) -> str: ...
diff --git a/stubs/isodate/py.typed b/stubs/isodate/py.typed
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/unit/backup/test_poll_timeout.py b/tests/unit/backup/test_poll_timeout.py
index c4cfdda86..9a0614038 100644
--- a/tests/unit/backup/test_poll_timeout.py
+++ b/tests/unit/backup/test_poll_timeout.py
@@ -12,9 +12,18 @@
 class TestPollTimeout:
     @pytest.mark.parametrize("it", ("PT0.999S", timedelta(milliseconds=999)))
     def test_min_validation(self, it: Union[str, timedelta]) -> None:
-        with pytest.raises(ValueError) as e:
+        with pytest.raises(
+            ValueError,
+            match=r"^Poll timeout must be at least one second, got: 0:00:00.999000$",
+        ):
             PollTimeout(it)
-        assert str(e.value) == "Poll timeout MUST be at least one second, got: PT0.999S"
+
+    def test_max_validation(self) -> None:
+        with pytest.raises(
+            ValueError,
+            match=r"^Poll timeout must be less than one year, got: 1 years, 0:00:00$",
+        ):
+            PollTimeout("P1Y")
 
     # Changing the default is not a breaking change, but the documentation needs to be adjusted!
     def test_default(self) -> None: