Skip to content
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

Odl palace update #1636

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Odl palace update #1636

wants to merge 3 commits into from

Conversation

leonardr
Copy link
Contributor

@leonardr leonardr commented Sep 3, 2021

Description

This branch brings in recent improvements to ODL collection management originated by @vbessonov in ThePalaceProject/circulation#25. It still needs some adaptation but the basic code should be in place now.

Motivation and Context

This is per https://jira.nypl.org/browse/SIMPLY-3868

How Has This Been Tested?

Checklist:

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@leonardr leonardr marked this pull request as draft September 3, 2021 20:49
@@ -596,7 +586,7 @@ def update_hold_queue(self, licensepool):
Loan.end>utc_now()
)
).count()
remaining_licenses = licensepool.licenses_owned - loans_count
remaining_licenses = max(licensepool.licenses_owned - loans_count, 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stops the number of licenses from going below zero.

now = util.datetime_helpers.utc_now()

if expires <= now:
continue
Copy link
Contributor Author

@leonardr leonardr Sep 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, this makes sure expires is a timezone-aware datetime. Second, this ensures we don't create any License objects for books whose licenses were already expired the first time we saw them. (The book itself will be created, but its LicensePool will have no Licenses.)

class MockODLAPI(ODLAPI):
"""Mock API for tests that overrides _get and _url_for and tracks requests."""

@classmethod
def mock_collection(self, _db):
def mock_collection(cls, _db, protocol=ODLAPI.NAME):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this right now -- it's for the ODL-via-OPDS-2 code that I didn't bring in -- but it doesn't hurt to have it.

feed = feedparser.parse(response_content)

return feed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just adds a little bit of error checking to the feed parsing process.

@leonardr
Copy link
Contributor Author

leonardr commented Sep 7, 2021

I ran this in the context of NYPL's QA server (where we happen to have a test ODL collection with some expired licenses), and watching it run made me concerned that the reaper algorithm only works in simple cases.
https://github.com/ThePalaceProject/circulation/pull/25/files#r703719081

I'm waiting to hear back before proceeding with this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant