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

Edition.add_bookcover() returning 500 response for valid urls #246

Open
xayhewalo opened this issue Jul 28, 2021 · 8 comments
Open

Edition.add_bookcover() returning 500 response for valid urls #246

xayhewalo opened this issue Jul 28, 2021 · 8 comments
Labels

Comments

@xayhewalo
Copy link
Contributor

Troubleshooting CoverBot revealed the following problem with add_bookcover

Python 3.9.4 (default, Apr  9 2021, 01:15:05) 
>>> from olclient.openlibrary import OpenLibrary
>>> ol = OpenLibrary()
>>> obj = ol.Edition.get('OL25511397M')
>>> obj.cover
>>> obj.ocaid
'elentremetidocom00gily'  # note the correct ocaid is elentremetidocom00gily_0
>>> obj.add_bookcover('https://archive.org/download/elentremetidocom00gily_0/page/cover')
<Response [500]>

https://archive.org/download/elentremetidocom00gily_0/page/cover is a valid url and the upload url used in add_bookcover resolves to https://openlibrary.org/books/OL25511397M/-/add-cover for this edition, which is also valid.

When I try to manually add a cover from the above upload url, I get an error. The url I land on is: https://openlibrary.org/books/OL25511397M/El_entremetido_comedia_en_tres_actos_n_!_prosa/add-cover
olclient-issue246

I suspect that root cause of this problem isn't with the client, but with OpenLibrary's backend, but I'm opening the issue here

@cdrini
Copy link
Collaborator

cdrini commented Jul 28, 2021

Actually, it looks like that book has been darked and is no longer available :/ https://archive.org/details/elentremetidocom00gily . Although I was able to upload the cover image by pasting the un-redirected url

@LeadSongDog
Copy link

LeadSongDog commented Jul 28, 2021

Seems the original file name on IA has changed to elentremetidocom00gily_0 (note the _0 suffixed) but the OL edition record did not get updated accordingly, which unprivileged users cannot do. This was commented in the OP by @xayhewalo but not remedied.

@cdrini Is there a way to check if this has happened on other records? It might explain some of the mysteriously dark links.

@cdrini
Copy link
Collaborator

cdrini commented Jul 28, 2021

Oh wow, good find! I would've never guessed the ocaid had changed. I'm not sure what could have caused that :/ Do you perchance know, @seabelis? I wonder if it might've also been a bug that might've stripped the _0 on import?

Ok, so then the upload issue is not with the Internet Archive record; that seems to be ok. The issue seems to be with uploading a URL like https://archive.org/download/elentremetidocom00gily_0/page/cover . I wonder if maybe Open Library is not following redirect? https://archive.org/download/elentremetidocom00gily_0/page/cover redirects to a different URL. We made a number of changes to this code when we switched it to use requests this past year; perhaps we're missing some flag to enable following redirects?

@cdrini
Copy link
Collaborator

cdrini commented Jul 28, 2021

(I fixed the IA ID)

@xayhewalo
Copy link
Contributor Author

@cdrini are those changes related to internetarchive/openlibrary#2852 ? Looks like a lot of PRs could potentially be the culprit

@cdrini
Copy link
Collaborator

cdrini commented Jul 30, 2021

Potentially. This is the code that gets called (I believe) when uploading via a url:

https://github.com/internetarchive/openlibrary/blob/3a4f8905dbc8426ef1d1ce26960227a9896d923b/openlibrary/coverstore/code.py#L144-L148

And that download function is here:

https://github.com/internetarchive/openlibrary/blob/3a4f8905dbc8426ef1d1ce26960227a9896d923b/openlibrary/coverstore/utils.py#L77-L78

Maybe this line was changed? Does requests.get follow redirects by default?

@cclauss
Copy link
Collaborator

cclauss commented Jul 30, 2021

https://docs.python-requests.org/en/latest/api/ says that requests.get(allow_redirects=True) is the default.

@LeadSongDog
Copy link

@hornc hornc added bug labels Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants