kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Update Django to version 3.2

Open rtibbles opened this issue 11 months ago • 24 comments

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 and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

rtibbles avatar Mar 07 '24 22:03 rtibbles

Hi @rtibbles, after a clean installation of the .deb build Kolibri is not loading the setup wizard. Logs and DB: logs.zip

pcenov avatar Mar 08 '24 15:03 pcenov

Ah, thanks @pcenov! Looks like none of the frontend files are being properly loaded - guessing Django is bypassing our previous setup somehow.

rtibbles avatar Mar 08 '24 15:03 rtibbles

Latest commit should have fixed this - updated assets will be along in a little while!

rtibbles avatar Mar 08 '24 15:03 rtibbles

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

pcenov avatar Mar 11 '24 10:03 pcenov

Who knew that changing everything would be so difficult?! :)

rtibbles avatar Mar 11 '24 15:03 rtibbles

Looks like there's some more issues with how we parse arguments for the import channel command (and probably the import content command too).

rtibbles avatar Mar 11 '24 15:03 rtibbles

@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.

rtibbles avatar Mar 12 '24 21:03 rtibbles

Hi @rtibbles, I found two additional issues while regression testing everything:

  1. The Android app can be installed but closes immediately after I launch it. Logs: android-logs.zip
  2. 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

pcenov avatar Mar 13 '24 16:03 pcenov

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.

rtibbles avatar Mar 13 '24 17:03 rtibbles

Some very interesting bugs in the other log, can take a closer look at them!

rtibbles avatar Mar 13 '24 17:03 rtibbles

Hi @rtibbles here's the logcat log: logcat.txt

pcenov avatar Mar 14 '24 11:03 pcenov

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!

rtibbles avatar Mar 14 '24 14:03 rtibbles

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.

rtibbles avatar Mar 14 '24 15:03 rtibbles

Hi @pcenov - I think I've addressed all the errors that I saw in the logs.

rtibbles avatar Mar 14 '24 15:03 rtibbles

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.

pcenov avatar Mar 18 '24 16:03 pcenov

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.

2024-03-18_17-10-59

Also no libraries are being displayed in the 'Other libraries' section of the Library page:

2024-03-19_12-01-33

logs.zip

pcenov avatar Mar 19 '24 15:03 pcenov

Thanks @pcenov - seems like the same underlying specific issue, I'll dig in and see what I can find.

rtibbles avatar Mar 19 '24 15:03 rtibbles

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.

rtibbles avatar Mar 19 '24 16:03 rtibbles

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.

rtibbles avatar Mar 21 '24 16:03 rtibbles

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.

rtibbles avatar Mar 22 '24 22:03 rtibbles

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:

  1. 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
  1. 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'

jredrejo avatar May 01 '24 18:05 jredrejo

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?

rtibbles avatar May 01 '24 18:05 rtibbles

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

jredrejo avatar May 01 '24 19:05 jredrejo

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.

rtibbles avatar May 03 '24 18:05 rtibbles

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.

rtibbles avatar May 03 '24 22:05 rtibbles