fix: add AsyncResponse type to niquests response object#439
Conversation
Signed-off-by: kyteinsky <kyteinsky@gmail.com>
📝 WalkthroughWalkthroughThe exception module is updated to support both synchronous and asynchronous response objects. Import statements, the base exception class, three specialized exceptions, and the public error-checking function all expand their response type annotations from ChangesException response type broadening
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@nc_py_api/_exceptions.py`:
- Line 3: The import may fail on older niquests releases because AsyncResponse
may not be re-exported; update the code to handle that by adding a fallback
import or pinning niquests: either pin niquests to a version known to export
AsyncResponse in your dependency manifest, or modify the import in
_exceptions.py to try the current top-level import of AsyncResponse, HTTPError,
Response and on ImportError fall back to the module path that actually defines
AsyncResponse (e.g., the internal async submodule) so the symbols AsyncResponse,
HTTPError, and Response are always available.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31cc61d6-8366-4c01-8b99-34108bbee9f6
📒 Files selected for processing (1)
nc_py_api/_exceptions.py
| """Exceptions for the Nextcloud API.""" | ||
|
|
||
| from niquests import HTTPError, Response | ||
| from niquests import AsyncResponse, HTTPError, Response |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is AsyncResponse exported from the top-level niquests package (e.g. from niquests import AsyncResponse)? What niquests version added it?
💡 Result:
Yes. Niquests exports AsyncResponse from the top-level package, so from niquests import AsyncResponse works, as reflected in Niquests’ API documentation which lists AsyncResponse as an autoclass in the main API docs alongside the top-level interface. [1] Version: I could not find a specific release note/tag that explicitly states “exported AsyncResponse” or the version where AsyncResponse was introduced. However, Niquests’ HISTORY notes that AsyncSession “now returns a AsyncResponse when stream is set to True” (i.e., the async streaming response type existed by the time of that change), but the HISTORY snippet retrieved does not include the version number for that change. [2]
Citations:
- 1: https://github.com/jawah/niquests/blob/main/docs/api.rst
- 2: https://github.com/jawah/niquests/blob/main/HISTORY.md
_exceptions.py: from niquests import AsyncResponse appears to be a supported top-level export—Niquests’ API docs list AsyncResponse for the main package namespace, so this import should resolve. Since no exact “introduced in version X” release note was found, consider pinning a specific niquests version (or adding a fallback import) to avoid an ImportError if older 3.x releases ever lacked the re-export.
🤖 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 `@nc_py_api/_exceptions.py` at line 3, The import may fail on older niquests
releases because AsyncResponse may not be re-exported; update the code to handle
that by adding a fallback import or pinning niquests: either pin niquests to a
version known to export AsyncResponse in your dependency manifest, or modify the
import in _exceptions.py to try the current top-level import of AsyncResponse,
HTTPError, Response and on ImportError fall back to the module path that
actually defines AsyncResponse (e.g., the internal async submodule) so the
symbols AsyncResponse, HTTPError, and Response are always available.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #439 +/- ##
=======================================
Coverage 94.67% 94.67%
=======================================
Files 48 48
Lines 5636 5636
=======================================
Hits 5336 5336
Misses 300 300
🚀 New features to boost your workflow:
|
1 participant
Changes proposed in this pull request:
AsyncResponse(child class ofResponse: https://github.com/jawah/niquests/blob/6dc75e944c41551a90ccabf08a203f1a719ce87b/src/niquests/models.py#L1660) if the request calls were made async-ly: https://github.com/jawah/niquests/blob/6dc75e944c41551a90ccabf08a203f1a719ce87b/src/niquests/async_session.py#L1150-L1182Summary by CodeRabbit