Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Thumbnail timeouts raise TimeoutError #695

Closed
1 task
AetherUnbound opened this issue May 11, 2022 · 2 comments · Fixed by #756
Closed
1 task

Thumbnail timeouts raise TimeoutError #695

AetherUnbound opened this issue May 11, 2022 · 2 comments · Fixed by #756
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🐍 tech: python Requires familiarity with Python

Comments

@AetherUnbound
Copy link
Contributor

Description

We recently changed the thumbnail service we're using and some of the business logic around it (#630). The previous thumbnail retrieval step did not have a timeout, whereas the new thumbnail retrieval step has a timeout of 5 seconds:

upstream_response = urlopen(req, timeout=5)

We should also add a case for the TimeoutError exception here:

except HTTPError as exc:

I think it might also be best for us to increase this timeout just a bit (maybe 10s?) to give the thumbnails for larger images/museum images more time to load.

Additional context

Sentry issue: https://sentry.io/share/issue/3daf5a2129a449859200e2de0c458ed6/

Resolution

  • 🙋 I would be interested in resolving this bug.
@AetherUnbound AetherUnbound added 🐍 tech: python Requires familiarity with Python 💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents labels May 11, 2022
@AetherUnbound AetherUnbound added good first issue New-contributor friendly help wanted Open to participation from the community labels May 11, 2022
@AetherUnbound
Copy link
Contributor Author

It seems we may also want to add RemoteDisconnected exceptions as well

@WordPress WordPress deleted a comment from sentry-io bot May 12, 2022
@WordPress WordPress deleted a comment from sentry-io bot May 12, 2022
@AetherUnbound
Copy link
Contributor Author

Perhaps we should have a broader exception clause in general? Here's a ConnectionResetError failure

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🐍 tech: python Requires familiarity with Python
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants