-
Notifications
You must be signed in to change notification settings - Fork 0
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
Ekir 219 handle 502 errors when loaning #134
Open
natlibfi-kaisa
wants to merge
8
commits into
main
Choose a base branch
from
EKIR-219-Handle-502-errors-when-loaning
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was raised after checking out a loan turned out not to be available in the api.
Required refacoring of how http responses are processed in general which required code changes in import related functionalities.
edefb05
to
7f3697f
Compare
7f3697f
to
c45d0e5
Compare
natlibfi-kaisa
commented
Feb 12, 2025
@@ -209,8 +209,6 @@ def _borrow(self, patron, credential, pool, mechanism): | |||
result = e.as_problem_detail_document(debug=False) | |||
except AuthorizationBlocked as e: | |||
result = e.as_problem_detail_document(debug=False) | |||
except CannotLoan as e: | |||
result = CHECKOUT_FAILED.with_debug(str(e)) |
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.
Tämä liittyy siihen syksyllä tehtyyn ongelmaan, jossa kirjaa ei ollutkaan saatavilla, kun käyttäjä yritti lainata hänelle varatun kirjan.
NatLibFi-JoonaKupe
approved these changes
Feb 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Handle the case where we think there are available licenses, but when we reach out to the content provider, it turns out there are no licenses available.
Motivation and Context
We were getting a lot of 502 responses with no specific debugging info. It turned out that the content provider was responding with a
application/api-problem+json
and not aapplication/problem+json
when there were no copies left when trying to checkout a book. The relevant code is inapi/odl.py
.This PR implements OPDS2 + ODL Handle case where no licenses are available solution together with their Refactor BadResponseException.
During the implementation, some needed refactoring of how HTTP responses in general were needed. These changes were done in
core/util/http.py
which had some minor changes needed inapi/overdrive.py
,customlists/customlist_import.py
,core/opds2_import.py
andcore/opds_import.py
.How Has This Been Tested?
Locally tested and unit tests.
Checklist