Optimise Library page load time when channels have large Thumbnail #12530

thesujai · 2024-08-02T08:17:12Z

Summary

As mentioned in the issue this PR does the following changes:

Defines a new API endpoint to get the channel_thumbnail as a img file
Changes the Current internal ChannelMetaData endpoint to return a URL to thumbnail instead directly returning a base64 encoded URL. This will cause async loading for thumbnails in the frontend

ScreenCast
Screencast from 02-08-24 01:44:04 PM IST.webm

References

Fixes #12502

Reviewer guidance

Open the library page and observe the channels load immediately, but the corresponding thumbnails loads asynchronously

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

…nail for internal channelmetadata

…ring

thesujai · 2024-08-02T08:22:46Z

kolibri/core/content/api.py

@@ -291,6 +294,14 @@ def filter_available(self, queryset, name, value):
        return queryset.filter(root__available=value)


+class ChannelThumbnailView(View):


For writing unit test for this is there any existing setUp method that will set up a channel metadata model?

If you look in the test_content_app.py file in the content app, you should see some examples.

thesujai · 2024-08-02T08:27:11Z

kolibri/plugins/learn/assets/src/views/ChannelThumbnail.vue

    computed: {
      thumbnailBackground() {
        return {
          backgroundColor: this.$themeTokens.surface,
-          backgroundImage: this.thumbnail ? `url('${this.thumbnail}')` : '',
+          backgroundImage: this.resolvedThumbnail ? `url('${this.resolvedThumbnail}')` : '',


I think we can do a high-level fix here, where we just modify the channels on the library page.
Now it is affecting all the cards on the library page. I don't know where to make changes for that.

github-actions · 2024-08-02T08:48:07Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.17.0a0.dev0_git.11.gc1417e39.pex
Windows Installer (EXE)	kolibri-0.17.0a0.dev0+git.11.gc1417e39-windows-setup-unsigned.exe
Debian Package	kolibri_0.17.0a0.dev0+git.11.gc1417e39-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.17.0a0.dev0+git.11.gc1417e39.dmg
Android Package (APK)	kolibri-0.17.0a0.dev0+git.11.gc1417e39-0.1.4-debug.apk
TAR file	kolibri-0.17.0a0.dev0+git.11.gc1417e39.tar.gz
WHL file	kolibri-0.17.0a0.dev0+git.11.gc1417e39-py2.py3-none-any.whl

rtibbles

This should work without any frontend changes, so let's aim for that.

Changes to the channel metadata endpoint can be simplified, and avoid code duplication.

I suspect that the reason the frontend changes were needed is because the backend endpoint is not returning something that the browser interprets as an image file.

rtibbles · 2024-08-05T13:48:37Z

kolibri/core/content/api.py

+        for item in items:
+            item["included_languages"] = included_languages.get(item["id"], [])
+            item["last_published"] = item["last_updated"]
+            item["thumbnail"] = reverse(


I think this might be doable using the field_map class property which allows setting to a callable, that would allow the rest of the code not to be copy pasted here.

rtibbles · 2024-08-05T13:50:31Z

kolibri/core/content/api.py

+    values = tuple(
+        field for field in BaseChannelMetadataMixin.values if field != "thumbnail"
+    )
+


So you would add here:

field_map = { "thumbnail": _create_channel_thumbnail_url, } field_map.update(BaseChannelMetadataMixin.field_map)

Then in the module scope you can define this function:

def _create_channel_thumbnail_url(item): return reverse("kolibri:core:channel-thumbnail", args=[item["id"]])

Yes this works, can you share django/python docs covering field_map?

This is not a Django thing - so the documentation for it is in this code comment: https://github.com/learningequality/kolibri/blob/develop/kolibri/core/api.py#L139

rtibbles · 2024-08-05T13:52:47Z

kolibri/plugins/learn/assets/src/views/ChannelThumbnail.vue

@@ -19,6 +19,7 @@
 <script>

  import ContentIcon from 'kolibri.coreVue.components.ContentIcon';
+  import { ChannelResource } from 'kolibri.resources';


If the ChannelThumbnail endpoint is working as intended, it should be returning a valid file if fetched - so you should just be able to use the URL directly as a file URL.

If this is not working, we need to make some tweaks, but when it is working correctly, this should require no frontend changes at all.

rtibbles · 2024-08-05T13:53:28Z

kolibri/core/content/api.py

@@ -291,6 +294,14 @@ def filter_available(self, queryset, name, value):
        return queryset.filter(root__available=value)


+class ChannelThumbnailView(View):


If you look in the test_content_app.py file in the content app, you should see some examples.

rtibbles · 2024-08-05T13:55:28Z

kolibri/core/content/api.py

+    def get(self, request, channel_id):
+        channel = get_object_or_404(models.ChannelMetadata, id=channel_id)
+        thumbnail = channel.thumbnail
+        response = FileResponse(thumbnail)


I am not confident this will be sufficient - the file is base64 encoded, which has a prefix with the mimetype. I think it is likely you would need to use the base64 decode method before serving this as the contents of a file: https://docs.python.org/3/library/base64.html#base64.b64decode

thesujai · 2024-08-05T15:45:17Z

@rtibbles can you take another look?

rtibbles

A couple of considerations, and a typo.

Have not yet manually tested!

kolibri/core/content/test/test_content_app.py

kolibri/core/content/api.py

…ts in the db

thesujai · 2024-08-05T19:56:14Z

I made the changes.
You can do the manual testing.
Now I can observe that when the library page loads, individual HTTP requests are made to get the thumbnail.
The Other libraries section which I suppose makes requests to the public endpoint is not affected by this change, as I see a base64 encoded thumbnail returned in the browser network tab. That is the desired behavior for now.

rtibbles

Looks good, after manual QA, this will be good to merge (after we've released 0.17.0).

Comments have all been addressed.

kolibri/core/content/api.py

radinamatic · 2024-08-07T14:52:06Z

cc @pcenov

pcenov · 2024-08-08T08:07:08Z

@radinamatic - no issues observed while manually testing, implemented as specified. Good to go!

rtibbles

Code changes look good, manual QA checks out, this is ready to go!

thesujai added 3 commits August 2, 2024 13:32

Add new api for fetching channel_thumnail and return the uri of thumb…

332c49d

…nail for internal channelmetadata

method to fetch thumbnail

213665c

handle case when thumbnail is an url and when thumnail is a base64 st…

c73e29f

…ring

github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend labels Aug 2, 2024

thesujai commented Aug 2, 2024

View reviewed changes

MisRob requested review from jredrejo and rtibbles August 2, 2024 09:09

rtibbles added this to the Kolibri 0.17: Planned Patch 1 milestone Aug 5, 2024

rtibbles previously requested changes Aug 5, 2024

View reviewed changes

thesujai added 2 commits August 5, 2024 20:35

use field_maps and decode the thumnail and serve it as aimg file

537242d

test for ChannelThumbnailView

e73ce95

rtibbles reviewed Aug 5, 2024

View reviewed changes

kolibri/core/content/test/test_content_app.py Outdated Show resolved Hide resolved

kolibri/core/content/api.py Outdated Show resolved Hide resolved

kolibri/core/content/api.py Outdated Show resolved Hide resolved

handle malformed thumbnails and return no url when thumbnail not exis…

c6ddc76

…ts in the db

rtibbles reviewed Aug 5, 2024

View reviewed changes

rtibbles reviewed Aug 6, 2024

View reviewed changes

kolibri/core/content/api.py Outdated Show resolved Hide resolved

jut get he mimetype from the header of thumbnail

519d50e

rtibbles approved these changes Aug 8, 2024

View reviewed changes

rtibbles merged commit 0b24c60 into learningequality:release-v0.17.x Aug 8, 2024
34 checks passed

rtibbles mentioned this pull request Aug 8, 2024

Channel thumbnails can cause extreme slowdown of channel API call #12502

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimise Library page load time when channels have large Thumbnail #12530

Optimise Library page load time when channels have large Thumbnail #12530

thesujai commented Aug 2, 2024 •

edited

Loading

thesujai Aug 2, 2024

rtibbles Aug 5, 2024

thesujai Aug 2, 2024

github-actions bot commented Aug 2, 2024 •

edited

Loading

rtibbles left a comment

rtibbles Aug 5, 2024

rtibbles Aug 5, 2024

thesujai Aug 5, 2024

rtibbles Aug 5, 2024

rtibbles Aug 5, 2024

rtibbles Aug 5, 2024

rtibbles Aug 5, 2024

thesujai commented Aug 5, 2024

rtibbles left a comment

thesujai commented Aug 5, 2024

rtibbles left a comment

radinamatic commented Aug 7, 2024

pcenov commented Aug 8, 2024

rtibbles left a comment

		@@ -291,6 +294,14 @@ def filter_available(self, queryset, name, value):
		return queryset.filter(root__available=value)


		class ChannelThumbnailView(View):

Optimise Library page load time when channels have large Thumbnail #12530

Optimise Library page load time when channels have large Thumbnail #12530

Conversation

thesujai commented Aug 2, 2024 • edited Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 2, 2024 • edited Loading

rtibbles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesujai commented Aug 5, 2024

rtibbles left a comment

Choose a reason for hiding this comment

thesujai commented Aug 5, 2024

rtibbles left a comment

Choose a reason for hiding this comment

radinamatic commented Aug 7, 2024

pcenov commented Aug 8, 2024

rtibbles left a comment

Choose a reason for hiding this comment

thesujai commented Aug 2, 2024 •

edited

Loading

github-actions bot commented Aug 2, 2024 •

edited

Loading