Add retry/backoff, timeout, and issue_id chunking to Redmine client

- #24: Mount HTTPAdapter with Retry(total=3, backoff_factor=0.5) for HTTP 429/500/502/503/504 on the Redmine session, set 30s timeout - #21: Split large issue_id lists into chunks of 100 to avoid exceeding URL length limits on reverse proxies Closes #24, #21
2026-06-27 13:06:41 +07:00
parent 14219564dd
commit 58fa5a7ab4
2 changed files with 144 additions and 11 deletions
--- a/redmine_reporter/client.py
+++ b/redmine_reporter/client.py
@@ -1,10 +1,18 @@
 from typing import Any, Dict, List, Optional, Tuple

+import requests
 from redminelib import Redmine
 from redminelib.resources import Issue
+from urllib3.util.retry import Retry

 from .config import Config

+# Таймаут на один HTTP-запрос к Redmine (секунды).
+REQUEST_TIMEOUT = 30
+
+# Размер чанка для запроса задач по issue_id, чтобы не превышать лимит длины URL (#21).
+ISSUE_ID_CHUNK_SIZE = 100
+

 def _get_redmine_auth_kwargs() -> Dict[str, Any]:
    """Return Redmine auth kwargs. API key has priority over legacy password auth."""
@@ -17,6 +25,51 @@ def _get_redmine_auth_kwargs() -> Dict[str, Any]:
    }


+def _make_retry_adapter() -> requests.adapters.HTTPAdapter:
+    """Создаёт HTTPAdapter с retry для временных ошибок (#24)."""
+    retry = Retry(
+        total=3,
+        backoff_factor=0.5,
+        status_forcelist=[429, 500, 502, 503, 504],
+        allowed_methods=["GET", "HEAD", "OPTIONS"],
+    )
+    return requests.adapters.HTTPAdapter(max_retries=retry)
+
+
+def _create_redmine() -> Redmine:
+    """Создаёт Redmine-клиент с таймаутом и retry-адаптером (#24)."""
+    redmine = Redmine(
+        Config.get_redmine_url(),
+        **_get_redmine_auth_kwargs(),
+        requests={
+            "verify": Config.get_redmine_verify(),
+            "timeout": REQUEST_TIMEOUT,
+        },
+    )
+
+    # Монтируем retry-адаптер на сессию для автоматических повторов
+    retry_adapter = _make_retry_adapter()
+    redmine.session.mount("https://", retry_adapter)
+    redmine.session.mount("http://", retry_adapter)
+
+    return redmine
+
+
+def _fetch_issues_chunked(
+    redmine: Redmine, issue_ids: List[int]
+) -> List[Issue]:
+    """Загружает задачи чанками, чтобы не превышать лимит длины URL (#21)."""
+    all_issues: List[Issue] = []
+    for i in range(0, len(issue_ids), ISSUE_ID_CHUNK_SIZE):
+        chunk = issue_ids[i : i + ISSUE_ID_CHUNK_SIZE]
+        issue_list_str = ",".join(str(x) for x in chunk)
+        issues = redmine.issue.filter(
+            issue_id=issue_list_str, status_id="*", sort="project:asc"
+        )
+        all_issues.extend(issues)
+    return all_issues
+
+
 def fetch_issues_with_spent_time(
    from_date: str, to_date: str
 ) -> Optional[List[Tuple[Issue, float]]]:
@@ -26,11 +79,7 @@ def fetch_issues_with_spent_time(
    Returns list of (issue, total_hours) tuples.
    """

-    redmine = Redmine(
-        Config.get_redmine_url(),
-        **_get_redmine_auth_kwargs(),
-        requests={"verify": Config.get_redmine_verify()},
-    )
+    redmine = _create_redmine()

    current_user = redmine.user.get("current")
    time_entries = redmine.time_entry.filter(
@@ -49,9 +98,9 @@ def fetch_issues_with_spent_time(
    if not issue_ids:
        return None

-    # Загружаем полные объекты задач
-    issue_list_str = ",".join(str(i) for i in issue_ids)
-    issues = redmine.issue.filter(issue_id=issue_list_str, status_id="*", sort="project:asc")
+    # Загружаем полные объекты задач чанками (#21)
+    sorted_ids = sorted(issue_ids)
+    issues = _fetch_issues_chunked(redmine, sorted_ids)

    # Сопоставляем задачи с суммарным временем.
    # Сортировка выполняется в report_builder.build_grouped_report,
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -133,7 +133,7 @@ def test_fetch_uses_api_key_when_present(mock_redmine_class):

    _, kwargs = mock_redmine_class.call_args
    assert kwargs["key"] == "api-token"
-    assert kwargs["requests"] == {"verify": DEFAULT_REDMINE_VERIFY}
+    assert kwargs["requests"]["verify"] == DEFAULT_REDMINE_VERIFY
    assert "username" not in kwargs
    assert "password" not in kwargs

@@ -151,7 +151,7 @@ def test_fetch_uses_username_password_when_no_api_key(mock_redmine_class):
    _, kwargs = mock_redmine_class.call_args
    assert kwargs["username"] == "user"
    assert kwargs["password"] == "password"
-    assert kwargs["requests"] == {"verify": DEFAULT_REDMINE_VERIFY}
+    assert kwargs["requests"]["verify"] == DEFAULT_REDMINE_VERIFY
    assert "key" not in kwargs


@@ -165,4 +165,88 @@ def test_fetch_uses_custom_verify_path(mock_redmine_class):
    fetch_issues_with_spent_time("2026-01-01", "2026-01-31")

    _, kwargs = mock_redmine_class.call_args
-    assert kwargs["requests"] == {"verify": "/tmp/redmine-ca.pem"}
+    assert kwargs["requests"]["verify"] == "/tmp/redmine-ca.pem"
+
+
+# -- #24: Таймаут и retry --
+
+
+@mock.patch.dict(os.environ, PASSWORD_ENV, clear=True)
+@mock.patch("redmine_reporter.client.Redmine")
+def test_fetch_sets_timeout_in_requests(mock_redmine_class):
+    """В requests dict передаётся timeout (#24)."""
+    mock_redmine = mock_redmine_class.return_value
+    _configure_current_user(mock_redmine)
+    mock_redmine.time_entry.filter.return_value = []
+
+    fetch_issues_with_spent_time("2026-01-01", "2026-01-31")
+
+    _, kwargs = mock_redmine_class.call_args
+    assert "timeout" in kwargs["requests"]
+    assert kwargs["requests"]["timeout"] == 30
+
+
+@mock.patch.dict(os.environ, PASSWORD_ENV, clear=True)
+@mock.patch("redmine_reporter.client.Redmine")
+def test_fetch_mounts_retry_adapter(mock_redmine_class):
+    """На сессию монтируется HTTPAdapter с retry для временных ошибок (#24)."""
+    mock_redmine = mock_redmine_class.return_value
+    _configure_current_user(mock_redmine)
+    mock_redmine.time_entry.filter.return_value = []
+
+    fetch_issues_with_spent_time("2026-01-01", "2026-01-31")
+
+    # Проверяем, что session.mount был вызван для http:// и https://
+    mount_calls = mock_redmine.session.mount.call_args_list
+    prefixes = [call.args[0] for call in mount_calls]
+    assert "https://" in prefixes
+    assert "http://" in prefixes
+
+    # Проверяем retry-конфигурацию адаптера
+    https_adapter = next(
+        call.args[1] for call in mount_calls if call.args[0] == "https://"
+    )
+    max_retries = https_adapter.max_retries
+    assert max_retries.total == 3
+    assert 429 in max_retries.status_forcelist
+
+
+# -- #21: Чанкирование issue_ids --
+
+
+@mock.patch.dict(os.environ, PASSWORD_ENV, clear=True)
+@mock.patch("redmine_reporter.client.Redmine")
+def test_fetch_chunks_large_issue_count(mock_redmine_class):
+    """При >100 задач запросы разбиваются на чанки (#21)."""
+    mock_redmine = mock_redmine_class.return_value
+    _configure_current_user(mock_redmine)
+
+    # 250 time entries → 250 уникальных issue_id
+    entries = []
+    for i in range(1, 251):
+        e = mock.MagicMock()
+        e.issue.id = i
+        e.hours = 1.0
+        entries.append(e)
+    mock_redmine.time_entry.filter.return_value = entries
+
+    # issue.filter вызывается с чанками по 100 ID
+    call_chunks = []
+
+    def issue_filter_side_effect(**kwargs):
+        ids_str = kwargs.get("issue_id", "")
+        call_chunks.append(ids_str)
+        ids = [int(x) for x in ids_str.split(",")]
+        return [mock.MagicMock(id=i, project="P", subject="T", status="New") for i in ids]
+
+    mock_redmine.issue.filter.side_effect = issue_filter_side_effect
+
+    result = fetch_issues_with_spent_time("2026-01-01", "2026-01-31")
+
+    # Должно быть 3 вызова (100 + 100 + 50)
+    assert len(call_chunks) == 3
+    assert len(call_chunks[0].split(",")) == 100
+    assert len(call_chunks[1].split(",")) == 100
+    assert len(call_chunks[2].split(",")) == 50
+    assert result is not None
+    assert len(result) == 250