Slow down a little when getting items from the HackerNews API #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to the way the HackerNews API works, getting items is a bit of a pain. There's no "get me the data for all these items please", it's "get me a list of IDs and them make me load every single on individually". To start with I just went all in on loading items up in parallel; which works fine for me but, eh, there are some situations where 500 simultaneous HTTP requests is a *bit* over the top (despite it being 2023! IKR?!?). So, in an initial effort to explore solutions for #2 without leaning into pagination, let's have a play with limiting the number of concurrent connections and see what works.
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The HackerNews API is... fun. There's no way of saying "get me the data for all of these stories please", all you can do is get one at a time. I'd initially done this concurrently which was pretty bloody fast, but also meant that when it came to things like "Top" or "New", you were making at least 500 concurrent HTTP requests.
Yes, I agree, in 2024 this should not be a problem and we should be allow to spam API endpoints with no consequence. Apparently we're not there yet.
So, in an effort to behave better1, this PR adds a configurable limit on the number of concurrent connections, and also adds a higher and configurable timeout on the connections too.
Super huge thanks to @mihaitodor for being one of those awesome folk who runs into a problem, comments about it on HackerNews, and then goes and lets the developer know by filing an issue. That's some faith-in-humanity-restored-level-shit right there.
I guess there's hope for 2024 yet!
All of which is to verbosely say... I think this helps fix #2 for now (although a more lazy method of loading things into the UI might be something for me to switch to longer-term).
Footnotes
In other words, do the thing I should have done to start with, if I wasn't paying way more attention to the UI rather than actually doing some proper coding on the tricky bits. ↩