-
Notifications
You must be signed in to change notification settings - Fork 7
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
Remove circulation api post processor (PP-501) #1470
Remove circulation api post processor (PP-501) #1470
Conversation
507c8e7
to
078ae8c
Compare
45eb58c
to
3e31953
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1470 +/- ##
==========================================
- Coverage 90.40% 90.40% -0.01%
==========================================
Files 230 231 +1
Lines 29693 29771 +78
Branches 6870 6880 +10
==========================================
+ Hits 26845 26914 +69
- Misses 1818 1823 +5
- Partials 1030 1034 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
This looks good!
There are a couple of places where it looks like there's a conflict with #1472, but they should be straightforward to resolve.
api/circulation.py
Outdated
return self.settings_class().construct( | ||
**self.integration_configuration().settings_dict | ||
) |
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.
Might need to resolve some changes from #1472 here when you merge/rebase main
.
tests/api/test_axis.py
Outdated
pytest.raises( | ||
ProblemError, MockAxis360API, axis360.db.session, axis360.collection | ||
) | ||
pytest.raises(ProblemError, Axis360Settings, **settings) |
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.
#1472 changes here, as well, I think.
21a3b3e
to
034d11e
Compare
Description
As a follow up to #1442, this removes the
CirculationFulfillmentPostProcessor
class. All the logic has been moved into the OPDSAPI and OPDS2API classes.Motivation and Context
CirculationFulfillmentPostProcessor
were only used to work around OPDS importers not having an API class. Now that we have a proper API class there, there is no reason to keep them around.How Has This Been Tested?
Running tests.
Checklist