search

skalnik / aqi-wtf

😷 WTF is the AQI near me right now?!
https://aqi.wtf
MIT License
44 stars 12 forks source link

Hitting rate limit often #22

Closed skalnik closed 4 years ago

skalnik commented 4 years ago

I've started to get more reports (and some anecdotal experience too) that the rate limit is being hit more regularly. Unsure if it's related to https://github.com/skalnik/aqi-wtf/pull/15 or something else. I think we can also move the refreshing up a bit, since I think the sensors just update once a minute based on the age being in minutes in the API.

obra commented 4 years ago

I was starting to see this well before #15. (It was what got me to go and implement #10.)

My suspicion is that PA tuned rate limits.

Is there anything we can do client-side to force caching of data.json so we're not refreshing it?

One possible idea is to use LocalStorage to cache the nearest sensor for a given latlon for 72 hours.

obra commented 4 years ago

Another idea might be to use GitHub Actions or Vercel to grab and cache data.json. Heck, it could get bundled into app.js. That'd let us cut it down to -just- the data we need to operate, saving a bunch of bandwidth for everybody.

obra commented 4 years ago

25 is a proposal for a possible solution to this.

skalnik commented 4 years ago

Yeah I think the lower refresh amount over in #23 helped 🙏 I'm definitely seeing it less than I was before.

I love the idea of #25 and think it's got some solid potential as a solution. Can probably implement some sort of expiration so we refetch if the last full list was found over an hour ago or something.

obra commented 4 years ago

On Sep 7, 2020, at 6:15 PM, Mike Skalnik notifications@github.com wrote:

 Yeah I think the lower refresh amount over in #23 helped 🙏 I'm definitely seeing it less than I was before.

I love the idea of #25 and think it's got some solid potential as a solution. Can probably implement some sort of expiration so we refetch if the last full list was found over an hour ago or something.

Sensors don’t come online very often. Why not cache for a few days?

I’d probably do the expiry as a second value with “last fetch” as a time stamp. We also want to clear the cache if the list appears missing or corrupted. Say “fewer than 50 sensors” in the list or something.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

skalnik commented 4 years ago

I'm always hesitant to cache longer than necessary, since if we introduce a problem it takes longer to resolve itself without any work on our end, but this is just more paranoia than any actual specific reason.

I'd maybe just pair them together as one JSON object stored under the same key, but either works. Definitely want to clear the cache if we have any trouble parsing it or anything! All great thinking.

obra commented 4 years ago

that’s a strong encouragement to include a cache version number in the cached data.

On Sep 7, 2020, at 7:27 PM, Mike Skalnik notifications@github.com wrote:

 I'm always hesitant to cache longer than necessary, since if we introduce a problem it takes longer to resolve itself without any work on our end, but this is just more paranoia than any actual specific reason.

I'd maybe just pair them together as one JSON object stored under the same key, but either works. Definitely want to clear the cache if we have any trouble parsing it or anything! All great thinking.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

obra commented 4 years ago

41 adds the caching. And...I can now refresh https://aqi-wtf-git-fork-obra-cache-sensor-list.skalnik.vercel.app/ every couple of seconds without hitting rate limit errors. So that's nice.

skalnik commented 4 years ago

Between #23 and #41 I haven't heard any complaints about this. If anyone hears more, we can reopen, but I'm counting this as closed for now!