Adjust response caching to consider all differences in API request params?

[jmobrien]

@juliasilge, thinking about #298--regardless of whether caching is relevant to that issue, caching itself is quite old, and predating a number of query-customization features. It's now easy to imagine ways that a cached response download could preempt a novel API call to fetch_survey(), even though that new call is actually asking for something quite different from what's cached.

Should this be fixed? Maybe by changing the IDs for cached files to include other arguments? Additionally or instead, perhaps by making force_request = FALSE the default?

[juliasilge]

I think the caching is quite surprising to most people, honestly, and changing the default would be a good way to go. I think that the caching can be a source of confusion and I don't know that it aligns with folks' expectations of how this function acts.

[jmobrien]

I avoid using it, myself, so I agree. Plus, it would remove the situation where response data (and thus, potentially, PII) becomes saved to disk in a not-directly-specified locale (the temp folder).

Would this qualify as a breaking change that we'd need to handle differently in publication?

[juliasilge]

Unfortunately there's not a great way to give messages to users when you change a default. Take a look at these options. We could do a message for everyone who uses the function ever, but that is very noisy.

One option we have is to change the default and document the heck out of it, but not add messages.

Another option that might be good to consider is to turn off the ability to cache altogether and deprecate the argument like this. This would give messages to anyone who passes in any value for force_request. Another option along those lines is to make a new argument for caching that replaces the old one, with the new one having the new default.

[jmobrien]

Thanks. Yes, this is challenging. I suppose we could try to thread the needle by adopting something like the first-run message approach you showed me--make force_request = FALSE the default, then give a first-run message about the new default if the user leaves force_request empty. Maybe an additional message if they explicitly set force_request = FALSE telling them that's now not needed. Would be less noisy I'd think--but still a decent quantity of total noise if those warning(s) get left active for a while.

I don't know. I think what I personally want is to remove it--I get the theory behind caching, but really question whether anyone uses the package in circumstances and at sufficient scale to see that benefit. Far more people probably find it annoying--I mean, if you're re-running a request, there's often a reason, right?

So maybe I'm leaning to the "remove caching entirely" option? My guess is that will be fine-to-welcome overall. If enough users later express problems with the change, we can put it back in under a new argument--perhaps with things like precise request matching and user control over where caches (and their potential PII) are saved?

[juliasilge]

So maybe I'm leaning to the "remove caching entirely" option? My guess is that will be fine-to-welcome overall. If enough users later express problems with the change, we can put it back in under a new argument--perhaps with things like precise request matching and user control over where caches (and their potential PII) are saved?

I like this plan a lot; it aligns with my own experience using the package and what we heard about most. 👍