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

Resize image files on perseus file export #4416

Open
4 tasks
rtibbles opened this issue Jan 24, 2024 · 24 comments · May be fixed by #4786
Open
4 tasks

Resize image files on perseus file export #4416

rtibbles opened this issue Jan 24, 2024 · 24 comments · May be fixed by #4786

Comments

@rtibbles
Copy link
Member

Observed behavior

Currently when inserting images into the answer section of a multiple choice question that gets exported into a Perseus format, if the images have been resized they have height and width dimensions attached to them, similarly to how they are annotated in the question block.

Unfortunately, Perseus ignores the height and width annotation when images are inserted into the answer section (rather than the question section).

Additionally, this also means that if someone inserted a very large image and then resized it to be significantly smaller, we would be shipping a much larger image than is necessary in the bundled perseus exercise.

Expected behavior

During the backend process to create a perseus exercise bundle, we should resize any images that have been resized, in order to bundle them at the appropriate resolution. This will solve both of the issues above.

User-facing consequences

This will fix several user reports of image insertion into answers that have not been properly resized in Kolibri, and also save our users valuable space!

Acceptance criteria

  • Read the width and height information for each image from the question data
  • Pass the width and height information for each image into the create_perseus_zip function - this could be done as a dict mapping from the image file name to the dimensions
  • Before the image file is written to the zip resize it according to the dimensions, using Python Image Library (PIL)
  • Add extra handling for the case where the same image file is reused in the same perseus file but with different dimensions - this is probably best done by renaming the image files with the checksum of the resized image and updating the exercise data with the relevant rename
@KshitijThareja
Copy link
Contributor

Hi @rtibbles. I would like to work on this issue. Could you please assign it to me if possible?
Thanks :)

@rtibbles
Copy link
Member Author

Hi @KshitijThareja - yes that would be fine - note that most of this should be relatively straight forward, but the edge case handling of the last item in the checklist is where it will be trickier (as it will require more interaction between the image resizing code and the code that is outputting the JSON data).

@KshitijThareja
Copy link
Contributor

Hey @rtibbles. I have a fair idea of what my approach should be. I just had one question. How do I test my changes? I am unsure if I can add a studio channel (from my local) containing a sample exercise to kolibri for testing it. And in studio itself, there's no preview available for the exercise.
I can only preview the quiz questions as shown here:
test

@rtibbles
Copy link
Member Author

It is possible to install a channel from your development Studio. You can either override the STUDIO url to use your development server on Kolibri using this environment variable: KOLIBRI_CENTRAL_CONTENT_BASE_URL or add it as another server.

I think it would also be a good idea to write some python unit tests for this code.

@KshitijThareja
Copy link
Contributor

Hey @rtibbles!
Sorry I had been a bit inactive here because of my exams.
While working on this issue, I had stumbled over another problem. I changed the CENTRAL_CONTENT_BASE_URL env variable to import contents from my local studio. I was able to see the published channels from my studio in kolibri's import channel section. But when I tried importing one of the available channels, I ran into an import error.

What's happening is when kolibri downloads the data for a specific channel, the data that is downloaded (the sqlite database) does not conform to the original database that should have been fetched, thereby giving a SchemaNotFound error. I am unsure how to proceed with this. Could you please double check this from your end?

@rtibbles
Copy link
Member Author

rtibbles commented Feb 17, 2024

Hrm, it should be configured to serve the files via a redirect: https://github.com/learningequality/studio/blob/unstable/contentcuration/contentcuration/dev_urls.py#L76

Can you post the SQlite database for the channel here as an attachment? You should be able to download it from 127.0.0.1:8080/content/databases/<channel_id>.sqlite3 from your Studio server.

@KshitijThareja
Copy link
Contributor

Yeah sure. Here's the Link.
Couldn't add it as an attachment directy.. This is the database that should be fetched on channel import. But the one that is actually fetched is empty.

@rtibbles
Copy link
Member Author

Hrm, I wonder if the redirect is causing issues when downloading, and hence causing the empty file.

Just to confirm, if you go to this URL: 127.0.0.1:8080/content/databases/66a31fdbdd745c7c9e499206f3c4cced.sqlite3 you get the file above downloaded?

If in a python shell you do:

import requests
response = requests.get("127.0.0.1:8080/content/databases/66a31fdbdd745c7c9e499206f3c4cced.sqlite3")
print(response.content)

Does it print the binary content of the file?

@KshitijThareja
Copy link
Contributor

KshitijThareja commented Feb 20, 2024

Does it print the binary content of the file?

Yeah, it does. I had checked it out earlier too.
I wasn't able to find any specific reason for the previously mentioned error though. It should be able to fetch the database correctly by default..

@KshitijThareja
Copy link
Contributor

Hi @rtibbles!
Sorry I wasn't able to pay attention to this issue because of my university exams. The error that we discussed about earlier while trying to import a channel still persists. What should be the next steps? I am unable to move forward with this issue currently.

@rtibbles
Copy link
Member Author

Can you confirm what you're setting your CENTRAL_CONTENT_BASE_URL to?

@KshitijThareja
Copy link
Contributor

I have set it to work with my local development server. The value is set to: "http://localhost:8080".

@rtibbles
Copy link
Member Author

Just in case there's a silly bug at play, could you try adding a trailing slash to the end? http://localhost:8080/ - I don't think this is the issue, but just want to double check.

@KshitijThareja
Copy link
Contributor

I tried that, still getting the same error. I wonder if it's the same for everyone, or if it's some misconfiguration from my end.

@rtibbles
Copy link
Member Author

The only other thing I can think of is that somehow the CENTRAL_CONTENT_BASE_URL is not properly getting used to fetch the channel database, and instead, it's attempting to fetch from Studio (so a bug in Kolibri).

Could you try adding the address as a "Peer import server" through the UI instead, and see if that works?

@KshitijThareja
Copy link
Contributor

Hi @rtibbles
I tried adding the same address (http://localhost:8080) as a Peer import server. It still gives the same error (SchemaNotFound error).
In both the cases, i.e. changing the CENTRAL_CONTENT_BASE_URL env variable's value to use the local studio server and using Peer import option, I am able to see the published channel in kolibri, but unable to download the resources because of the aforementioned error.

@rtibbles
Copy link
Member Author

I will test locally and see if this is a more systematic issue, or if there is some configuration that is still missing for you!

@MisRob
Copy link
Member

MisRob commented Sep 3, 2024

@KshitijThareja would you like to return to this or should we unassign?

I just checked with @rtibbles and it seems it should be possible to continue if you'd like to:

The fix for the local import [from a local studio instance] should be fixed on Kolibri develop, so testing should be possible now

@KshitijThareja
Copy link
Contributor

Hi @MisRob!
Sorry I was occupied with some other tasks. I'll get back to working on it.
Thanks :)

@MisRob
Copy link
Member

MisRob commented Sep 3, 2024

Thanks @KshitijThareja and no pressure at all, take all the time you wish (I'm just doing my regular status and clarification rounds :)

@KshitijThareja
Copy link
Contributor

KshitijThareja commented Oct 12, 2024

Hi @MisRob, @rtibbles!
Sorry for being late with the update regarding this issue. I was a little occupied with some work, and it was taking a while to figure out the exact solution for this issue. I tried a lot of approaches, and it'd have been done some time ago if I had realized one small mistake that I have been making.
Nevertheless, I just wanted to inform that I will be making a PR for it in a day or two, just have to do some code cleanup and refactoring. Apart from that, I'm pretty much done with the solution 😄

@KshitijThareja KshitijThareja linked a pull request Oct 13, 2024 that will close this issue
24 tasks
@MisRob
Copy link
Member

MisRob commented Oct 14, 2024

Hi @KshitijThareja, good to hear from you :) No pressure, thanks a lot!

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

@KshitijThareja Hi Kshitij, seems this is on hiatus, but just in case you'd return to it, note that we will be closed from December 23 to January 5.

@KshitijThareja
Copy link
Contributor

Hi @MisRob!
I am currently busy with some academic commitments. Will return to this just after my exams.
Thanks and happy holidays :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants