-
Notifications
You must be signed in to change notification settings - Fork 798
Not declaring OpenAlex missing a DOI if 429'd #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the OpenAlex integration by improving error handling, test coverage, and timeout configuration. The changes address specific timeout issues observed with certain DOIs and add support for distinguishing between DOIs that exist but aren't open access versus DOIs that don't exist in OpenAlex.
- Updated error handling to specifically catch 404 responses and provide more detailed error messages
- Extended test coverage to include more OpenAlex scenarios (open access, non-open access, and not found cases)
- Added timeout configuration to the test client to handle slow API responses
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/paperqa/clients/openalex.py | Enhanced error handling to distinguish 404s from other HTTP errors, removed unused import, added comment explaining timeout issues |
| tests/test_clients.py | Expanded test parameters to cover more scenarios (in OpenAlex with/without OA, not in OpenAlex) and added timeout configuration |
| tests/cassettes/*.yaml | Added VCR cassettes for new test cases capturing API responses |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR also:
ConnectTimeout