Avoid expensive caller stack inspection - #96
Conversation
📝 WalkthroughWalkthroughThe queue now resolves caller names through direct frame access instead of full stack inspection. Tests verify exact caller names and ChangesQueue caller lookup
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@tests/test_client_queue.py`:
- Around line 324-341: Add AttributeError coverage for _get_caller_name
alongside the existing ValueError unavailable-frame tests, preferably by
parameterizing the shared cases with both exception types. Ensure each case
configures the mocked sys._getframe to raise the selected exception and verifies
the fallback result remains "Unknown".
- Around line 324-341: Update both _BadSys test doubles in
tests/test_client_queue.py with Google-style class docstrings, and change each
_getframe return annotation from Any to Never or NoReturn since it always raises
ValueError. Add the corresponding typing import if needed, then run the
repository-local .venv checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ca0424e-9f7f-412f-83e7-23aba078df29
📒 Files selected for processing (2)
aiopnsense/client_queue.pytests/test_client_queue.py
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Avoids a full Python stack inspection for every queued API request, removing a disproportionate recurring CPU cost during normal polling.
What Changed
inspect.stack()caller lookup with a direct frame lookup.Unknowndiagnostic fallback when the caller frame is unavailable.Why
The former lookup walks every stack frame and triggers costly source/module resolution despite the label being used only for diagnostics. The queue call depth is fixed, so a direct frame lookup preserves the label without that work.
Related Issues
Closes #94
Summary by CodeRabbit
Bug Fixes
"Unknown"fallback when caller information is unavailable.Tests
Greptile Summary
This PR replaces full stack inspection with direct frame lookup for diagnostic caller labels while preserving the
"Unknown"fallback.sys._getframe(3)to avoid repeated full-stack traversal.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
inspect.stack()with fixed-depthsys._getframe()lookup and updates the unavailable-frame fallback exceptions.Reviews (2): Last reviewed commit: "test: cover queue caller frame fallbacks" | Re-trigger Greptile