search

endlessm / kolibri-explore-plugin

The kolibri plugin to add the custom channel representation
MIT License
2 stars 4 forks source link

Fix channel view with Kolibri 0.16.0-beta6 #873

Closed dbnicholson closed 11 months ago

dbnicholson commented 11 months ago

The Kolibri metadata_cache decorator is tied to the content lifetime, but that has nothing to do with the channel app data. Furthermore, metadata_cache in Kolibri 0.16.0-beta6 is broken for this usage since regular View classes don't have render functions.

Instead, simply use the cache_control decorator to set max-age=600. This caches the responses for 10 minutes. Even though these are likely very static, I don't want to set these longer for now since we have no control over when the apps bundle gets updated.

Fixes: https://github.com/endlessm/kolibri-explore-plugin/issues/872

dbnicholson commented 11 months ago

Looks good! Should we add a comment to link the upstream issue?

We could, but I didn't actually file it yet 😜. And actually when I thought about it a little more, I think I understand why it's that way. That decorator is for content API views. That means they're rest framework viewsets that always have render functions.

We were completely misusing it for the side effect of a long cache control max age. So, it might be nice to have it work for generic views or throw a nicer exception, but I think it could very easily be wontfixed, too.

dbnicholson commented 11 months ago

I opened https://github.com/learningequality/kolibri/issues/11362 now, so I'll add that to the commit message. I also think we can do very long caching after looking closer at the URLs. They always contain the explore plugin version, so it should be safe to use very long caching so long as we always update the apps bundle with the plugin.

dbnicholson commented 11 months ago

@manuq I updated the commit message and bumped the max age up to 1 week. Do you want to take a look or should I merge? I'd like to get this released today since I believe both the Flatpak and the Android app are on 0.16.0-beta6 and currently broken.

dbnicholson commented 11 months ago

I'm merging so I can make a release.