kolibri
kolibri copied to clipboard
Update Django to version 3.2
Summary
- Updates all Django related dependencies to ones compatible with 3.2
- Updates translation and language code previously vendored from Django for modification
- Updates all url function invocations to re_path
- Updates filterset to filterset_class for DRF viewsets
- Use the new
header
attribute for response objects to access header values - Explicitly mark tests that access more than one of the test databases with
__all__
- Updates tests that were sending
None
as a value for a POST - the Django test client no longer allows this - Changes
is_authenticated
check from method invocation to property check - Fixes imports
- Changes function signatures for field methods
- Updates how we serialize and deserialize JSONFields and DateTimeTzFields to match change in Morango - now both serialization and deserialization behaviour are explicitly flagged - parallel to how Django's own serialization framework handles it except for specific reserved types
- Adds the now required
--all
argument to the makemessages command - Updates static lib usage
- Adds setting to set the automatic PK field to AutoIncrementingInteger
- Stops deletion of querysets that have had
distinct
called on them - Removes python 2 unicode decorator
- Cleans up all swappable dependency cruft from migrations for FacilityUser
- Update Django model fields based on warnings/errors from Django
- Removes bugfixes that are now in Django
- Handle badly formed GET parameters now raising 400s
- Update how we do inheritance for anonymous user and facility user, as anonymous users can no longer be an abstract Model class
- For some bulk operations (channel import, db restore) turn off SQLite referential integrity checks, as they are stricter than PostgreSQL and get verified at statement execution, not transaction commit.
- Update postres and cryptography dependencies
- Removes collections and translations monkey patching that was previously required
- Updates html5lib to not need collections monkey patching
References
Fixes #11726
Reviewer guidance
The urls migration could have been done more selectively by using path
instead of re_path
in cases where regex is not strictly needed, but that seemed like a task better suited for follow up.
The database attribute of test classes could also have been more selectively applied, but it was easier to use __all__
.
Do tests pass?
I didn't update any documentation, as I don't think anything hugely changed.
Testing checklist
- [x] 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
- [x] Critical and brittle code paths are covered by unit tests
PR process
- [x] PR has the correct target branch and milestone
- [x] 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
andpip
) - Documentation is updated
- Contributor is in AUTHORS.md
Build Artifacts
Asset type | Download link |
---|---|
PEX file | kolibri-.pex |
Windows Installer (EXE) | kolibri-0.16.2b1.dev0+git.1034.g6f7db014-windows-setup-unsigned.exe |
Debian Package | kolibri_0.16.2b1.dev0+git.1034.g6f7db014-0ubuntu1_all.deb |
Mac Installer (DMG) | kolibri-0.16.2b1.dev0+git.1034.g6f7db014-0.4.1.dmg |
Android Package (APK) | kolibri-0.16.2b1.dev0+git.1034.g6f7db014-0.1.2-debug.apk |
TAR file | kolibri-0.16.2b1.dev0+git.1034.g6f7db014.tar.gz |
WHL file | kolibri-0.16.2b1.dev0+git.1034.g6f7db014-py2.py3-none-any.whl |
Hi @rtibbles, after a clean installation of the .deb build Kolibri is not loading the setup wizard. Logs and DB: logs.zip
Ah, thanks @pcenov! Looks like none of the frontend files are being properly loaded - guessing Django is bypassing our previous setup somehow.
Latest commit should have fixed this - updated assets will be along in a little while!
Hi @rtibbles now the Setup Wizard is working correctly but it's not possible import a channel in any of the supported ways:
https://github.com/learningequality/kolibri/assets/79847249/943602e1-aba6-4a7e-933a-e7a6ae862df9
Logs and DB: logs.zip
Who knew that changing everything would be so difficult?! :)
Looks like there's some more issues with how we parse arguments for the import channel command (and probably the import content command too).
@pcenov this should now be fixed. I also looked for other possible instances of the same error. If you could also test the facility import, single user import + syncing, on my own setup + merge to single user syncing, and facility syncing that should cover the places that I updated.
Hi @rtibbles, I found two additional issues while regression testing everything:
- The Android app can be installed but closes immediately after I launch it. Logs: android-logs.zip
- When migrating a user, the process gets stuck at "Remotely integrating data". If I sign out and sign in again with the same user I can see that the user is actually migrated. Logs: migrate-user-logs.zip
https://github.com/learningequality/kolibri/assets/79847249/4bfa49e9-cf6e-479d-944c-ce6aec71106c
Hi @pcenov - the android app kolibri log is exceptionally short. Could you rerun the app and grab the logcat output? That might give me some more clues.
Some very interesting bugs in the other log, can take a closer look at them!
Hi @rtibbles here's the logcat log: logcat.txt
Thank you @pcenov - hah, I think this might be a result of our not having merged 0.16.x into develop recently enough rather than anything I've done here, so I'll get that sorted too. Thank you!
Oh, on further investigation, that doesn't seem to be the issue - it just looks very similar to an error we had late on in the 0.16.0 release process.
Hi @pcenov - I think I've addressed all the errors that I saw in the logs.
Hi @rtibbles - I confirm that the Android app installs and runs correctly now. I'm still testing the critical workflows with all installers and will let you know if I find anything else that's not working.
Hi @rtibbles the only new issues I was able to identify are for the Mac app. There I am getting a server error at Device > Channels > Select a source
which remains in a 'Loading connections' state and it's not possible to import resources.
Also no libraries are being displayed in the 'Other libraries' section of the Library page:
Thanks @pcenov - seems like the same underlying specific issue, I'll dig in and see what I can find.
This appears to be an issue with how the certifi
library is getting bundled in our Mac App. I am not entirely sure why the Django upgrade has triggered this, but it starts to get a bit byzantine in terms of how it is sorting out its dependency inspection.
I think I will file this as a follow up issue, as I don't think I can fix it within the scope of this PR.
Thinking in KDP, this migration: content/../0036_drop_null_boolean_field.py will take hours, looking at its code it looks like model changes are a Django version requirement but they look compatible with the current db columns, or have you checked if there's some column that needs to be actually changed in the db?
I doubt anything needs to change in the DB, so it would be nice to trick Django into thinking we've done this if possible. I'll look into what we can do.
This diff...
I have implemented most of these changes. The admin app has been removed, and the pagination ordering warning has been resolved - I did not make the edit to the PublicFacilityUserViewset, as it doesn't use pagination.
While addressing this, I also found a little bit of cruft in our settings file, and noticed that I hadn't updated any links to documentation for the new Django version.
content/../0036_drop_null_boolean_field.py
I renamed this migration file to better reflect what it is doing, as it is dropping the null boolean field and updating the mptt fields to positive integers.
When I ran some testing, the null boolean fields were no-ops in both SQLite and Postgresql, producing no SQL queries (similarly for the update to the JSONField). The positive integer constraint and index changes from mptt were more impactful in SQLite, but had almost no impact on a similarly sized Postgresql database.
All that to say, this shouldn't cause a long migration for Postgresql DBs, and I don't know how to make it quicker for SQLite without modifying Django's migration behaviour horribly.
First of all, I tried to test it in KDP and I gave up, models and urls must be changed in KDP for it to work. I consider that, if other kolibri plugins are working with Django 3.2 it should work as well.
When testing it with a non-existing kolibri installation, I could not start this code, this is the traceback, no matter if I start it using kolibri start --foreeground
or yarn python-devserver
:
ERROR 2024-05-01 20:37:37,392 Job 0 raised an exception: Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 423, in execute
return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: no such table: morango_databaseidmodel
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/datos/le/mio/kolibri/kolibri/core/tasks/job.py", line 326, in execute
result = func(*args, **kwargs)
File "/datos/le/mio/kolibri/kolibri/core/tasks/registry.py", line 237, in __call__
return self.func(*args, **kwargs)
File "/datos/le/mio/kolibri/kolibri/core/analytics/tasks.py", line 27, in _ping
ping_once(started, server=server)
File "/datos/le/mio/kolibri/kolibri/core/analytics/utils.py", line 468, in ping_once
data = perform_ping(started, server=server)
File "/datos/le/mio/kolibri/kolibri/core/analytics/utils.py", line 420, in perform_ping
instance, _ = InstanceIDModel.get_or_create_current_instance()
File "/usr/lib/python3.10/contextlib.py", line 79, in inner
return func(*args, **kwds)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/morango/models/core.py", line 160, in get_or_create_current_instance
database_id=DatabaseIDModel.get_or_create_current_database_id().id
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/morango/models/core.py", line 93, in get_or_create_current_database_id
return cls.objects.get(current=True)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 431, in get
num = len(clone)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 262, in __len__
self._fetch_all()
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1324, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 51, in __iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1175, in execute_sql
cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 66, in execute
return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
return executor(sql, params, many, context)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 79, in _execute
with self.db.wrap_database_errors:
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 423, in execute
return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: no such table: morango_databaseidmodel
ERROR 2024-05-01 20:37:37,402 Error in 'RUN' listener <bound method ZeroConfPlugin.RUN of <kolibri.utils.server.ZeroConfPlugin object at 0x7fdfba14cdf0>>
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 423, in execute
return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: no such table: morango_databaseidmodel
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/magicbus/base.py", line 273, in publish
result = listener(*args, **kwargs)
File "/datos/le/mio/kolibri/kolibri/utils/server.py", line 331, in RUN
instance = build_broadcast_instance(self.port)
File "/datos/le/mio/kolibri/kolibri/core/discovery/utils/network/broadcast.py", line 271, in build_broadcast_instance
device_info = get_device_info()
File "/datos/le/mio/kolibri/kolibri/core/device/utils.py", line 461, in get_device_info
instance_model = InstanceIDModel.get_or_create_current_instance()[0]
File "/usr/lib/python3.10/contextlib.py", line 79, in inner
return func(*args, **kwds)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/morango/models/core.py", line 160, in get_or_create_current_instance
database_id=DatabaseIDModel.get_or_create_current_database_id().id
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/morango/models/core.py", line 93, in get_or_create_current_database_id
return cls.objects.get(current=True)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 431, in get
num = len(clone)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 262, in __len__
self._fetch_all()
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1324, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/query.py", line 51, in __iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1175, in execute_sql
cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 66, in execute
return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
return executor(sql, params, many, context)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 79, in _execute
with self.db.wrap_database_errors:
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/utils.py", line 90, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/django/db/backends/sqlite3/base.py", line 423, in execute
return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: no such table: morango_databaseidmodel
INFO 2024-05-01 20:37:37,422 Bus state: START_ERROR
ERROR 2024-05-01 20:37:37,423 Exiting due to error in start listener:
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/magicbus/base.py", line 215, in _transition
return self.publish(newstate, *args, **kwargs)
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/magicbus/base.py", line 291, in publish
raise exc
magicbus.base.ChannelFailures: OperationalError('no such table: morango_databaseidmodel')
When using it with an existing kolibri installation, code works and I have not been able to appreciate any problem in the frontend. Backend complains in some cases with warnings, not with complete errors, kolibri never stops. This is the list of warnings that I have spotted:
- For these models:
- TasksViewSet
- NetworkLocationFacilitiesView
- ImportMetadataViewset
- ProgressTrackingViewSet
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/rest_framework/generics.py", line 63, in get_queryset
assert self.queryset is not None, (
AssertionError: 'TotalContentProgressViewSet' should either include a `queryset` attribute, or override the `get_queryset()` method
- For these models:
- ContentNodeProgressViewset
- FreeSpaceView
- NetworkLocationFacilitiesView
- ProgressTrackingViewSet
- TotalContentProgressViewSet
- ImportMetadataViewset
AttributeError: 'ContentNodeGranularViewset' object has no attribute 'channel_stats'
WARNING 2024-05-01 20:30:01,497 view's ContentNodeProgressViewset raised exception during schema generation; use `getattr(self, 'swagger_fake_view', False)` to detect and short-circuit this
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/drf_yasg/inspectors/base.py", line 42, in call_view_method
return view_method()
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/rest_framework/generics.py", line 108, in get_serializer
serializer_class = self.get_serializer_class()
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/rest_framework/generics.py", line 122, in get_serializer_class
assert self.serializer_class is not None, (
AssertionError: 'ContentNodeProgressViewset' should either include a `serializer_class` attribute, or override the `get_serializer_class()` method.
WARNING 2024-05-01 20:30:01,271 view's ClassroomNotificationsViewset raised exception during schema generation; use `getattr(self, 'swagger_fake_view', False)` to detect and short-circuit this
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/drf_yasg/inspectors/base.py", line 42, in call_view_method
return view_method()
File "/datos/le/mio/kolibri/kolibri/plugins/coach/api.py", line 183, in get_queryset
classroom_id = self.kwargs["classroom_id"]
KeyError: 'classroom_id'
WARNING 2024-05-01 20:30:01,468 view's ContentNodeGranularViewset raised exception during schema generation; use `getattr(self, 'swagger_fake_view', False)` to detect and short-circuit this
Traceback (most recent call last):
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/drf_yasg/inspectors/base.py", line 42, in call_view_method
return view_method()
File "/datos/le/mio/kolibri/venv/lib/python3.10/site-packages/rest_framework/generics.py", line 109, in get_serializer
kwargs.setdefault('context', self.get_serializer_context())
File "/datos/le/mio/kolibri/kolibri/core/content/api.py", line 1483, in get_serializer_context
context.update({"channel_stats": self.channel_stats})
AttributeError: 'ContentNodeGranularViewset' object has no attribute 'channel_stats'
I don't think the warnings are a problem, but the errors when trying to start kolibri in a clean installation are critical.
Odd that @pcenov didn't get any of these issues - must be something to do with a dev environment and a clean KOLIBRI_HOME directory specifically, I guess?
I don't think the warnings are a problem, but the errors when trying to start kolibri in a clean installation are critical.
Odd that @pcenov didn't get any of these issues - must be something to do with a dev environment and a clean KOLIBRI_HOME directory specifically, I guess?
I've tried it again and discovered that one comment that I must have typed by accident in main.py was blocking migrations. Sorry for the noise.
After trying it again the errors avoiding it to start have disappeared. With this running instance I've also tested importing a facility and the result was fine. In the middle of the process I can see these random errors in the console:
ERROR 2024-05-01 21:11:05,473 ContentSyncHook.post_transfer hook failed
Traceback (most recent call last):
File "/datos/le/mio/kolibri/kolibri/core/discovery/utils/network/client.py", line 127, in request
raise requests.exceptions.ConnectionError("No socket available")
requests.exceptions.ConnectionError: No socket available
that might indicate sockets being exhusted, but I don't think this is related to the new Django version.
The warnings mentioned in the previous comment are there. We should decide how important they're before proceeding or not. They all seem pretty easy to fix though
Yeah - I think they are worth cleaning up, but again, I think they're probably specifically in development mode, related to the API schema generation, but would be good to add some defensive checks in there to prevent the warnings.
I have added defensive checks and some dummy methods where appropriate to prevent the warnings being displayed on the api explorer page, so I think this should be ready to go.