fix: add AsyncResponse type to niquests response object#439
Conversation
Signed-off-by: kyteinsky <kyteinsky@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe exception module now accepts both synchronous and asynchronous response objects in its base exception, specialized exceptions, and error-checking helper. The project dependency constraint for ChangesException response type broadening
Dependency version pin
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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:
|
_exceptions.py now imports AsyncResponse, which niquests first exposed as a top-level export in 3.4.2 (absent in 3.0.0 to 3.4.1). The previous niquests>=3 floor would let `import nc_py_api` fail with ImportError on older niquests, so raise the floor to match. Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
2 participants
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
check_error.niquestsversion requirement to3.4.2(still under the same major-version cap).