-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Tidy up some extraneous / out-of-date archive.org references #9438
base: master
Are you sure you want to change the base?
Conversation
remove irrelevant field `ia_box_id` from test data relates to internetarchive#7465
testing for it has no effect `lendinglibrary` is not a collection that exists on any archive.org item. https://archive.org/search?query=collection%3Alendinglibrary Gives zero results
(['lendinglibrary'], ['Lending library'], {}, 'lendable'), | ||
(['lendinglibrary'], ['Some other subject'], {}, 'restricted'), | ||
(['inlibrary'], ['Lending library'], {}, 'lendable'), | ||
(['inlibrary'], ['Some other subject'], {}, 'restricted'), | ||
(['inlibrary'], ['In library'], {}, 'restricted'), |
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 test is now failing, but I don't understand what it is testing.
lendinglibrary
doesn't exist as a collection.
Neither Lending library
or In library
appear to exist on Open Library in any quantity
https://openlibrary.org/search/subjects?q=Lending+library&mode=everything
https://openlibrary.org/search/subjects?q=In+Library&mode=everything
If they did, I'd consider them fake subjects, and suggest they be cleaned up.
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.
The one concrete thing I thought this was used for is setting the value of 'preview'
in
https://openlibrary.org/api/books.json?bibkeys=
responses. EDIT: No, that's done in a more straighforward manner in book/dynlinks.py
Two restricted examples:
https://openlibrary.org/api/books.json?bibkeys=059035342X,0312368615
From the recently returned carousel:
https://openlibrary.org/api/books.json?bibkeys=1598220209
but it's no longer up, and the preview access states full
?
Full is the fallback option an doesn't capture removed items.
Unrestricted:
https://openlibrary.org/api/books.json?bibkeys=lccn:13025071
preview: full
seems correct.
Borrowable book:
https://openlibrary.org/api/books.json?bibkeys=1565847032
preview: borrow
What is the difference between dynlinks.py and readlinks.py?
both are used in openlibrary/plugins/books/code.py
Looks like readlinks.py is the 'Read API' (now called the Partner API? )
https://openlibrary.org/dev/docs/api/read
The doc page states the values for status
are:
'status'
'full access', 'lendable', 'checked out' or 'restricted.'
and the code suggests these are possible values:
openlibrary/openlibrary/plugins/books/readlinks.py
Lines 259 to 265 in 8b0f5ec
statusvals = { | |
'full access': 1, | |
'lendable': 2, | |
'checked out': 3, | |
'restricted': 4, | |
'restricted - not inlib': 4, | |
'missing': 5, |
for more information, see https://pre-commit.ci
# def is_lending_library(self): | ||
# collections = self.get_ia_collections() | ||
# return 'lendinglibrary' in collections | ||
return 'printdisabled' in v |
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 like it should be
return 'printdisabled' in v or 'inlibrary' in v
But this will not have been working correctly for some time.
Is this method still current?
It is used in this template here: https://github.com/internetarchive/openlibrary/blob/8b0f5ecdd1cd7e7088c19ac8e3e1d60eca1a95eb/openlibrary/templates/books/daisy.html
I don't know how to access this from the current site.
This FAQ page is still linked: https://openlibrary.org/help/faq/accessing
and AFAIK DAISY is still relevant and usable?
Two possible courses of action:
- Fix this code if it is still relevant
- Delete this method and the corresponding template if it is not current.
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.
Raised #9439 for clearing up what is required for DAISY.
Relates to #7465
ia_box_id
is used for somethingTechnical
I'm not touching any Solr related code (see #7465) , just removing core code which has no effect.
Testing
Screenshot
Stakeholders
@cdrini
@mekarpeles