Skip to content

feat: allow per-call GET request timeouts - #24

Merged
tnware merged 2 commits into
mainfrom
feat/get-request-timeout-override
May 26, 2026
Merged

tnware merged 2 commits into
mainfrom
feat/get-request-timeout-override

Conversation

@tnware

@tnware tnware commented May 26, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Adds a per-call timeout override to invoke_get_rest_api_call(...)
  • Reuses the selected timeout for both the initial GET and any retry after re-authentication
  • Covers the override with a regression test

Test Plan

  • python -m pytest tests/test_request_timeout.py::test_get_requests_allow_per_call_timeout_override -q — failed before implementation with TypeError
  • python -m pytest tests/test_request_timeout.py -q — pass
  • python -m pytest -q — pass
  • ruff check . — pass
  • python -m pip install -e '.[dev,docs]' — pass
  • pytest -q — pass
  • python -m build — pass
  • twine check dist/* — pass
  • sphinx-build -E -W -b html docs docs/_build/html — pass
  • git diff --check — pass

Summary by CodeRabbit

  • New Features

    • API calls now support per-request timeout overrides, enabling you to specify custom timeout values for individual requests without affecting the default configuration.
  • Tests

    • Added test to verify per-request timeout overrides work correctly across all request scenarios.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 26, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@tnware, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 55 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d57d50da-00c5-49f5-b98b-32495c6263f0

📥 Commits

Reviewing files that changed from the base of the PR and between 63aeb95 and 0858194.

📒 Files selected for processing (1)
  • tests/test_request_timeout.py
📝 Walkthrough

Walkthrough

This PR adds per-call timeout override support to the REST API client. The invoke_get_rest_api_call method now accepts an optional timeout parameter that overrides the client's configured request_timeout. The effective timeout is computed once and reused in both the initial GET request and any subsequent retry calls after a 401 re-authentication flow.

Changes

Per-call timeout override

Layer / File(s) Summary
Method signature and contract
unifi_controller_api/api_client.py
invoke_get_rest_api_call signature extended with optional timeout parameter. Docstring updated to document that per-call timeout overrides the client-level request_timeout.
Timeout override implementation
unifi_controller_api/api_client.py
Effective timeout is computed once from the timeout argument (or client's request_timeout) and passed to requests.Session.get in both the initial request and 401 retry loop paths.
Test validation
tests/test_request_timeout.py
New test test_get_requests_allow_per_call_timeout_override verifies that per-call timeout override is respected by the underlying session call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A timeout that bends to thy will,
Per-call, per-request, just as you will,
Initial and retry, both hear the same call—
No more hardcoded waits, freedom for all! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding per-call timeout support to GET requests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/get-request-timeout-override

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/test_request_timeout.py (1)

75-84: ⚡ Quick win

Add a retry-path assertion for timeout reuse.

This test validates the first GET call timeout, but not the 401 re-auth retry path that was also changed. Please add a case that forces a 401 then verifies the retried session.get(...) still uses the same per-call timeout override.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_request_timeout.py` around lines 75 - 84, Update the
test_get_requests_allow_per_call_timeout_override to also exercise the 401
re-auth retry path: configure the fake session used by make_controller to return
a 401 response first and a successful 200 on the retry (so
controller.invoke_get_rest_api_call triggers the retry), call
invoke_get_rest_api_call with timeout=2.5 as before, and then assert that the
retried call recorded in controller.session.get_calls[1][1]["timeout"] is also
2.5 (i.e., verify the second get call preserves the per-call timeout override).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_request_timeout.py`:
- Around line 75-84: Update the
test_get_requests_allow_per_call_timeout_override to also exercise the 401
re-auth retry path: configure the fake session used by make_controller to return
a 401 response first and a successful 200 on the retry (so
controller.invoke_get_rest_api_call triggers the retry), call
invoke_get_rest_api_call with timeout=2.5 as before, and then assert that the
retried call recorded in controller.session.get_calls[1][1]["timeout"] is also
2.5 (i.e., verify the second get call preserves the per-call timeout override).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6929cbb-2c8e-452f-be19-27ae77e9778a

📥 Commits

Reviewing files that changed from the base of the PR and between 824af80 and 63aeb95.

📒 Files selected for processing (2)
  • tests/test_request_timeout.py
  • unifi_controller_api/api_client.py

@tnware

tnware commented May 26, 2026

Copy link
Copy Markdown
Owner Author

Addressed in 0858194.

Added test_retried_get_request_keeps_per_call_timeout_override, which forces the first GET to return 401, lets the client re-authenticate, and verifies both the initial and retried session.get(...) calls use the per-call timeout override ([2.5, 2.5]).

Validation:

  • python -m pytest tests/test_request_timeout.py::test_retried_get_request_keeps_per_call_timeout_override -q
  • ruff check .
  • python -m pytest -q
  • python -m build
  • twine check dist/*
  • sphinx-build -E -W -b html docs docs/_build/html
  • git diff --check

@tnware
tnware merged commit 0446e67 into main May 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant