Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix channel view with Kolibri 0.16.0-beta6 #873

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

dbnicholson
Copy link
Member

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: #872

Copy link
Collaborator

@manuq manuq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@dbnicholson
Copy link
Member Author

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
Copy link
Member Author

I opened learningequality/kolibri#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.

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[1].

Instead, simply use the `cache_control` decorator to set the max age for
a week. Since these URLs always contain the plugin version (and the
files in the channel zips are generally cache busting), it should be
safe to essentially never expire the responses.

Fixes: #872

1. learningequality/kolibri#11362
@dbnicholson dbnicholson force-pushed the 872-broken-metadata-cache branch from 8bd96bc to 619e349 Compare October 6, 2023 15:13
@dbnicholson
Copy link
Member Author

@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
Copy link
Member Author

I'm merging so I can make a release.

@dbnicholson dbnicholson merged commit 71fd48a into master Oct 6, 2023
@dbnicholson dbnicholson deleted the 872-broken-metadata-cache branch October 6, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Channel view broken with Kolibri 0.16.0-beta6
2 participants