kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Upgrade Python dependencies

Open rtibbles opened this issue 9 months ago • 12 comments

Summary

  • Upgrades all Python dependencies to the latest version that supports Python 3.6
  • Note that it does not upgrade the Python redis library as far as it could go, because the django-redis-cache library we are using requires a redis version less than 4

References

Because we are still supporting EOL Python versions, we have to do these upgrades manually rather than relying on dependabot

Reviewer guidance

First we check if the tests pass, then a smoke test of the asset is in order, particularly testing Redis caching.


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

rtibbles avatar May 13 '24 23:05 rtibbles

The only way to test that would be alongside kolibri-server - failing that, when this is tested in KDP.

rtibbles avatar Jun 12 '24 15:06 rtibbles

Hi @rtibbles sorry but I am still not sure how to properly test this one - could you please provide additional testing guidance?

pcenov avatar Jun 13 '24 07:06 pcenov

As @marcellamaki mentioned - I think just smoke testing the various assets here is about all that is needed, so just making sure they all start (bar the Mac installer, which we have a separate issue filed for) - and ensuring that you can access Kolibri.

rtibbles avatar Jun 13 '24 14:06 rtibbles

Hi @rtibbles

  1. The Android app is force closing immediately after being launched:

android_logs.zip

  1. Both the .deb and .exe assets can be installed but it's not possible to import a facility and to preview a lesson resource when creating lessons:

https://github.com/learningequality/kolibri/assets/79847249/5ab5f396-b029-43e4-886f-70aced55769a

2024-06-14_14-54-11

ubuntu_logs.zip

pcenov avatar Jun 14 '24 12:06 pcenov

Seems like this is less straightforward than I had hoped!

rtibbles avatar Jun 14 '24 22:06 rtibbles

Hi @pcenov, I have rebased onto the latest develop, which should fix the 500 error you saw in import, and have made a fix for the error I saw in the Android logs!

rtibbles avatar Jun 26 '24 00:06 rtibbles

Hrm, interesting - I'm not seeing anything unusual in the logs, my only hunch is that this has added a lot of additional files, causing the unpacking to take longer.

rtibbles avatar Jun 27 '24 17:06 rtibbles

Yeah, sadly that seems to be the case - it just takes a lot of time for the unpacking to happen...

pcenov avatar Jun 28 '24 07:06 pcenov

I can confirm this as replicable on my devices too. If there's nothing we can do to speed up the unpacking time, can we at least make sure to have the Kolibri loader instead of the blank screen for the duration? That would be better UX...

radinamatic avatar Jun 28 '24 08:06 radinamatic

Or as we mentioned it with @pcenov, straight-up and explicit Loading... (we have the string).

radinamatic avatar Jun 28 '24 15:06 radinamatic

Looking at the assets, it looks like we now have two different packages that provide a complete copy of the timezone database, I think this is the cause of the extra files.

I think we can clean this up and only have one of these packages, but it seems simpler to do this after 0.17.0 is released. Bumping this to the first patch milestone instead.

rtibbles avatar Jul 01 '24 15:07 rtibbles

Hi @rtibbles - I've tested again all the installers without any regressions observed, should be good to go!

pcenov avatar Aug 09 '24 15:08 pcenov