bug-1809726: update to django 4.2 #6538

willkg · 2024-02-29T02:32:33Z

This updates Socorro from Django 3.2 LTS to 4.2 LTS. This is a more involved update than Tecken was because Socorro uses more of Django's stuff for the web ui whereas Tecken is predominantly a react app.

This starts with updating Django which then required me to update django-pipeline. That unearthed some problems one of which was fixed by adjusting the bundle specification. The other required us to switch from one jsonview js lib which isn't maintained to another one which sort of is.

On top of that, Django deprecated a bunch of stuff that were were doing with the plan of removing those things in Django 5.0. So I ran django-upgrade and I also fixed some things by hand.

To test:

run make build which builds everything and also builds the static assets
run the tests
run the webapp, process some crash reports, click around
1. check report view "Raw data and minidumps" and "Telemetry Environment" tabs which use the jsonview library
2. check the admin

willkg · 2024-02-29T02:32:58Z

docker/Dockerfile

@@ -31,7 +31,7 @@ COPY --chown=app:app ./webapp/package*.json /webapp-frontend-deps/
 RUN cd /webapp-frontend-deps/ && npm install

 COPY --chown=app:app requirements.txt /app/
-RUN pip install -U 'pip==23.3.1' && \
+RUN pip install -U 'pip==24.0' && \


I got tired of the pip warning, so I updated pip, too.

willkg · 2024-02-29T02:34:45Z

webapp/crashstats/api/tests/test_views.py

@@ -110,7 +110,7 @@ def test_base_classes_raise_not_found(self):
    def test_option_CORS(self):
        """OPTIONS request for model_wrapper returns CORS headers"""
        url = reverse("api:model_wrapper", args=("NoOp",))
-        response = self.client.options(url, HTTP_ORIGIN="http://example.com")
+        response = self.client.options(url, headers={"origin": "http://example.com"})


The Django test client takes a headers argument instead of headers as kwargs. I don't know which Django version changed this--it's possible it's been this way for a while and we never updated.

willkg · 2024-02-29T02:35:21Z

webapp/crashstats/authentication/admin.py

@@ -38,12 +38,12 @@ class UserAdminBetter(UserAdmin):
        "get_policyexception",
    ]

+    @admin.display(description="Hacker (PII)")


Instead of setting properties on the methods, there's a decorator that you can use to set things. We switched to that.

https://docs.djangoproject.com/en/5.0/ref/contrib/admin/#django.contrib.admin.display

willkg · 2024-02-29T02:39:46Z

webapp/crashstats/crashstats/admin.py

        "object_link",
        "action",
        "get_change_message",
    ]
    list_display_links = ["action_time", "get_change_message"]

-    def admin(self, obj):
+    def admin_email(self, obj):


We needed to change this method name because it overrides admin in the class namespace preventing us from using admin.display() decorator.

willkg · 2024-02-29T02:41:11Z

webapp/crashstats/crashstats/static/crashstats/css/components/message.less

@@ -24,9 +24,9 @@ div#message {
 }
 .message.warning {
    background-color: @pink;
-    border: 1px solid @jsonview-bool;
+    border: 1px solid @orange;


I got rid of jsonview-bool when I updated the CSS for the jsonview. It's weird that the messages system was using that color. It was kind of a lighter tangerine color, so I replaced it with orange.

willkg · 2024-02-29T02:42:31Z

webapp/crashstats/crashstats/tests/test_views.py

@@ -201,7 +202,7 @@ def test_html(self):
        handler500 = getattr(views, end)

        # to make a mock call to the django view functions you need a request
-        fake_request = RequestFactory().request(**{"wsgi.input": None})
+        fake_request = RequestFactory().request(**{"wsgi.input": StringIO("")})


Turns out when you pass None in as a value, something in Django internals calls .read() on it and then everything gets mad. wsgi.input should be a stream, so I replaced it with a StringIO and that seemed to work.

willkg · 2024-02-29T02:43:09Z

webapp/crashstats/settings/base.py

-# No need to load it because we don't do i18n in this project
-USE_I18N = False
-
-USE_L10N = False


These are deprecated. I think that means Django always has localization enabled. We don't use that in Crash Stats, so I think this is probably fine and if not, someone will tell us and we can figure out what to do then.

willkg · 2024-02-29T02:43:51Z

webapp/crashstats/settings/base.py

@@ -470,6 +465,12 @@ def filter(self, record):
 DATABASES = {"default": dj_database_url.parse(DATABASE_URL)}


+STORAGES = {


This used to be STATICFILES_STORAGE and is now just STORAGES with a different structure.

https://docs.djangoproject.com/en/5.0/releases/4.2/#custom-file-storages

willkg · 2024-02-29T02:44:30Z

webapp/crashstats/settings/bundles.py

@@ -34,12 +34,15 @@
    "@fortawesome/fontawesome-free": ["css/all.min.css", "webfonts/*"],
    "tablesorter": ["dist/css/theme.default.min.css", "dist/js/jquery.tablesorter.js"],
    "d3": ["dist/*"],
-    "jssha": ["dist/*.js"],
+    "jssha": ["dist/*.js", "dist/*.map"],


The .js file has an indicator to the .map file and something uses that now (I have no idea what), so we need to copy over the .map file now.

willkg · 2024-02-29T02:45:45Z

webapp/crashstats/settings/bundles.py

+    "jquery.json-viewer": [
+        "json-viewer/jquery.json-viewer.css",
+        "json-viewer/jquery.json-viewer.js",
+    ],


jquery-jsonview has a file that indicates there's a .map file just like jssha did, but there is no .map file in the package. The library hasn't been touched in 8 years. So I switched us to jquery.json-viewer which has some minor differences, but it's been updated more recently and doesn't have the .map problem.

willkg · 2024-02-29T02:46:55Z

webapp/crashstats/supersearch/form_fields.py

@@ -206,7 +206,7 @@ class IsoDateTimeField(forms.DateTimeField):
    def to_python(self, value):
        if value:
            try:
-                return parse_isodate(value).replace(tzinfo=utc)
+                return parse_isodate(value).replace(tzinfo=datetime.timezone.utc)


django.utils.timezone.utc is deprecated in Django 4.1 in favor of datetime.timezone.utc which it was an alias for since Django 4.0.

http://docs.djangoproject.com/en/5.0/releases/4.1/

willkg · 2024-02-29T02:49:14Z

webapp/crashstats/tokens/jinja2/tokens/home.html

@@ -115,7 +115,7 @@ <h2>Generate a New Token</h2>
        <form method="post">
          {% csrf_token %}
          <table class="data-table">
-            {{ form }}
+            {{ form.as_table() }}


Django project created a new div form renderer that's accessibility friendly which the table form renderer is not. Calling {{ form }} will change what it's doing from the table form renderer it does now to the div form renderer. So we call form.as_table() here because I didn't want to spend time on redoing the API token form using the div form renderer and hunt for other forms that need to be similarly updated.

I'll write up a bug to cover that work.

http://docs.djangoproject.com/en/5.0/releases/4.1/

https://bugzilla.mozilla.org/show_bug.cgi?id=1883044

This updates Django to 4.2. In order to do that, we also had to update django-pipeline to 3.0.0 so it worked correctly. But then that caused problems when building static files. I managed to fix the jssha error, but couldn't fix the jquery.jsonview one. jquery.jsonview is unmaintained and hasn't had a version release in 8 years and the example urls redirect to terrible places. I switched Crash Stats to use jquery.json-viewer which required some minor js and css changes, but generally works the same.

This fixes a bunch of DeprecationWarnings introduced with Django 4.1 and 4.2 for things that are going away in Django 5.0. * change in how forms are rendered -- I picked `as_table()` for now since that's what it used to do; we want to switch to the new accessible-friendly form rendering--I'll write up a bug to do that * deprecating django.utils.timezone.utc -- these are all switched to datetime.timezone.utc * pass in headers as headers dict to test client * switch setting admin method properties to use a decorator This also fixes a new bug where RequestFactory `wsgi.input` value needs a `read()` method.

willkg · 2024-03-01T14:16:36Z

Thank you!

willkg requested a review from a team as a code owner February 29, 2024 02:32

willkg requested a review from relud February 29, 2024 02:32

willkg commented Feb 29, 2024

View reviewed changes

relud approved these changes Feb 29, 2024

View reviewed changes

willkg added 2 commits March 1, 2024 08:47

willkg force-pushed the willkg-bug-1809726-django42 branch from e9cecf8 to 7869d5d Compare March 1, 2024 14:02

willkg merged commit c99dc11 into main Mar 1, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug-1809726: update to django 4.2 #6538

bug-1809726: update to django 4.2 #6538

willkg commented Feb 29, 2024 •

edited

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024 •

edited

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Feb 29, 2024

willkg Mar 1, 2024

willkg commented Mar 1, 2024

		@@ -470,6 +465,12 @@ def filter(self, record):
		DATABASES = {"default": dj_database_url.parse(DATABASE_URL)}


		STORAGES = {

bug-1809726: update to django 4.2 #6538

bug-1809726: update to django 4.2 #6538

Conversation

willkg commented Feb 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willkg Feb 29, 2024 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willkg commented Mar 1, 2024

willkg commented Feb 29, 2024 •

edited

willkg Feb 29, 2024 •

edited