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

bug-1809726: update to django 4.2 #6538

Merged
merged 2 commits into from Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docker/Dockerfile
Expand Up @@ -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' && \
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

pip install --no-cache-dir --no-deps -r requirements.txt && \
pip check --disable-pip-version-check

Expand Down
6 changes: 3 additions & 3 deletions requirements.in
Expand Up @@ -9,7 +9,7 @@ django-cors-headers==4.3.1
django_csp==3.7
django-jinja==2.11.0
django-npm==1.0.0
django-pipeline==2.1.0
django-pipeline==3.0.0
django-ratelimit==4.1.0
djangorestframework==3.14.0
django-session-csrf==0.7.1
Expand Down Expand Up @@ -63,8 +63,8 @@ sphinx_rtd_theme==2.0.0
# remove it.
django-waffle==2.3.0

# NOTE(willkg): We stick with LTS releases and the next one is 4.2 (2023).
django==3.2.24
# NOTE(willkg): We stick with LTS releases and the next one is 5.2 (2025).
django==4.2.10

# NOTE(willkg): Need to keep elasticsearch and elasticsearch-dsl at these versions
# because they work with the cluster we're using
Expand Down
16 changes: 7 additions & 9 deletions requirements.txt
Expand Up @@ -261,9 +261,9 @@ dj-database-url==2.1.0 \
--hash=sha256:04bc34b248d4c21aaa13e4ab419ae6575ef5f10f3df735ce7da97722caa356e0 \
--hash=sha256:f2042cefe1086e539c9da39fad5ad7f61173bf79665e69bf7e4de55fa88b135f
# via -r requirements.in
django==3.2.24 \
--hash=sha256:5dd5b787c3ba39637610fe700f54bf158e33560ea0dba600c19921e7ff926ec5 \
--hash=sha256:aaee9fb0fb4ebd4311520887ad2e33313d368846607f82a9a0ed461cd4c35b18
django==4.2.10 \
--hash=sha256:a2d4c4d4ea0b6f0895acde632071aff6400bfc331228fc978b05452a0ff3e9f1 \
--hash=sha256:b1260ed381b10a11753c73444408e19869f3241fc45c985cd55a30177c789d13
# via
# -r requirements.in
# dj-database-url
Expand All @@ -287,9 +287,9 @@ django-jinja==2.11.0 \
django-npm==1.0.0 \
--hash=sha256:2e6bba65e728fa18b9db3c8dc0d4490b70cb7f43bacf60eb3654d7dcb6424272
# via -r requirements.in
django-pipeline==2.1.0 \
--hash=sha256:36a6ce56fdf1d0811e4d51897f534acca35ebb35be699d9d0fd9970e634792a4 \
--hash=sha256:e91627faee22c4c65eb7d134ef53a9d97253c99e4dd914af8ea9c8c58c01de93
django-pipeline==3.0.0 \
--hash=sha256:49a8bee298668100bb6e8a2144dff8c607baa5297820a2503793c38693f34103 \
--hash=sha256:e9e08b084ef3ebf599795510519a8d44f2240b487782bebf4a8fcaf6302c31d1
# via -r requirements.in
django-ratelimit==4.1.0 \
--hash=sha256:555943b283045b917ad59f196829530d63be2a39adb72788d985b90c81ba808b \
Expand Down Expand Up @@ -767,9 +767,7 @@ python-decouple==3.8 \
pytz==2023.3.post1 \
--hash=sha256:7b4fddbeb94a1eba4b557da24f19fdf9db575192544270a9101d8509f9f43d7b \
--hash=sha256:ce42d816b81b68506614c11e8937d3aa9e41007ceb50bfdcb0749b921bf646c7
# via
# django
# djangorestframework
# via djangorestframework
pyyaml==6.0.1 \
--hash=sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5 \
--hash=sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc \
Expand Down
20 changes: 11 additions & 9 deletions webapp/crashstats/api/tests/test_views.py
Expand Up @@ -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"})
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

assert response.status_code == 200
assert response["Access-Control-Allow-Origin"] == "*"

Expand Down Expand Up @@ -158,14 +158,14 @@ def test_hit_or_not_hit_ratelimit(self):
# https://bugzilla.mozilla.org/show_bug.cgi?id=1148470
for _ in range(current_limit * 2):
response = self.client.get(
url, {"product": "good"}, HTTP_X_REAL_IP="12.12.12.12"
url, {"product": "good"}, headers={"x-real-ip": "12.12.12.12"}
)
assert response.status_code == 429

# But it'll work if you use a different X-Real-IP
# because the rate limit is based on your IP address
response = self.client.get(
url, {"product": "good"}, HTTP_X_REAL_IP="11.11.11.11"
url, {"product": "good"}, headers={"x-real-ip": "11.11.11.11"}
)
assert response.status_code == 200

Expand All @@ -175,7 +175,7 @@ def test_hit_or_not_hit_ratelimit(self):
)

response = self.client.get(
url, {"product": "good"}, HTTP_AUTH_TOKEN=token.key
url, {"product": "good"}, headers={"auth-token": token.key}
)
assert response.status_code == 200

Expand Down Expand Up @@ -255,7 +255,9 @@ def mocked_get(**params):
token = Token.objects.create(user=user, notes="test token")
token.permissions.add(view_pii_perm)

response = self.client.get(url, {"crash_id": "123"}, HTTP_AUTH_TOKEN=token.key)
response = self.client.get(
url, {"crash_id": "123"}, headers={"auth-token": token.key}
)
assert response.status_code == 200
dump = json.loads(response.content)
for key in public_data.keys():
Expand Down Expand Up @@ -326,7 +328,7 @@ def mocked_get(**params):
token.permissions.add(view_pii_perm)

response = self.client.get(
url, {"crash_id": "abc123"}, HTTP_AUTH_TOKEN=token.key
url, {"crash_id": "abc123"}, headers={"auth-token": token.key}
)
assert response.status_code == 200
dump = json.loads(response.content)
Expand Down Expand Up @@ -548,7 +550,7 @@ def mocked_publish(queue, crash_ids):
assert response.status_code == 403

params = {"crash_ids": crash_id}
response = self.client.get(url, params, HTTP_AUTH_TOKEN="somecrap")
response = self.client.get(url, params, headers={"auth-token": "somecrap"})
assert response.status_code == 403

user = User.objects.create(username="test")
Expand All @@ -560,10 +562,10 @@ def mocked_publish(queue, crash_ids):
token = Token.objects.create(user=user, notes="Only reprocessing")
token.permissions.add(perm)

response = self.client.get(url, params, HTTP_AUTH_TOKEN=token.key)
response = self.client.get(url, params, headers={"auth-token": token.key})
assert response.status_code == 405

response = self.client.post(url, params, HTTP_AUTH_TOKEN=token.key)
response = self.client.post(url, params, headers={"auth-token": token.key})
assert response.status_code == 200
assert json.loads(response.content) is True

Expand Down
6 changes: 2 additions & 4 deletions webapp/crashstats/authentication/admin.py
Expand Up @@ -38,12 +38,12 @@ class UserAdminBetter(UserAdmin):
"get_policyexception",
]

@admin.display(description="Hacker (PII)")
Copy link
Collaborator Author

@willkg willkg Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

def is_hacker(self, obj):
"""Return whether user is in the Hackers group and has access to PII."""
return obj.groups.filter(name="Hackers").exists()

is_hacker.short_description = "Hacker (PII)"

@admin.display(description="PolicyException")
def get_policyexception(self, obj):
"""Return whether user has a PolicyException with links to change/delete or add one."""
if hasattr(obj, "policyexception"):
Expand All @@ -57,8 +57,6 @@ def get_policyexception(self, obj):
)
return format_html('<a href="{}">Create</a>', url)

get_policyexception.short_description = "PolicyException"


@admin.register(PolicyException)
class PolicyExceptionAdmin(admin.ModelAdmin):
Expand Down
18 changes: 8 additions & 10 deletions webapp/crashstats/crashstats/admin.py
Expand Up @@ -30,14 +30,14 @@ class LogEntryAdmin(admin.ModelAdmin):

list_display = [
"action_time",
"admin",
"admin_email",
"object_link",
"action",
"get_change_message",
]
list_display_links = ["action_time", "get_change_message"]

def admin(self, obj):
def admin_email(self, obj):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return obj.user.email

def action(self, obj):
Expand All @@ -51,6 +51,10 @@ def obj_repr(self, obj):
return edited_obj.email
return edited_obj

@admin.display(
description="object",
ordering="object_repr",
)
def object_link(self, obj):
object_link = self.obj_repr(obj) # Default to just name
content_type = obj.content_type
Expand All @@ -69,14 +73,10 @@ def object_link(self, obj):
pass
return object_link

object_link.admin_order_field = "object_repr"
object_link.short_description = "object"

@admin.display(description="change message")
def get_change_message(self, obj):
return obj.get_change_message()

get_change_message.short_description = "change message"

def has_add_permission(self, request):
return False

Expand Down Expand Up @@ -127,6 +127,7 @@ class SignatureAdmin(admin.ModelAdmin):
search_fields = ["signature"]


@admin.action(description="Process crashes")
def process_crashes(modeladmin, request, queryset):
"""Process selected missing processed crashes from admin page."""
priority_api = PriorityJob()
Expand All @@ -137,9 +138,6 @@ def process_crashes(modeladmin, request, queryset):
)


process_crashes.short_description = "Process crashes"


@admin.register(MissingProcessedCrash)
class MissingProcessedCrashAdmin(admin.ModelAdmin):
list_display = [
Expand Down
Expand Up @@ -43,4 +43,3 @@
@dark-gray: @dark-grey;

@jsonview-prop: #fcb35f;
@jsonview-bool: #f89962;
Expand Up @@ -24,9 +24,9 @@ div#message {
}
.message.warning {
background-color: @pink;
border: 1px solid @jsonview-bool;
border: 1px solid @orange;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
.message h2 {
margin: .5em 0;
font-size: 1.2em;
}
}
Expand Up @@ -15,7 +15,7 @@ $(document).ready(function () {
if (container.length) {
var jsonData = container.data('telemetryenvironment');
try {
$('#telemetryenvironment-json').JSONView(jsonData);
$('#telemetryenvironment-json').jsonViewer(jsonData, { withLinks: false });
} catch (ex) {
console.warn('The data in the #telemetryenvironment-json dataset is not valid JSON');
container.append($('<p>Error when parsing JSON. Showing raw value:</p>'));
Expand All @@ -38,7 +38,7 @@ $(document).ready(function () {
if (container.length) {
var jsonData = container.data('minidumpstackwalk');
try {
$('#minidump-stackwalk-json').JSONView(jsonData);
$('#minidump-stackwalk-json').jsonViewer(jsonData, { withLinks: false });
} catch (ex) {
console.warn('The data in the #minidump-stackwalk-json dataset is not valid JSON');
container.append($('<p>Error when parsing JSON. Showing raw value:</p>'));
Expand Down
56 changes: 10 additions & 46 deletions webapp/crashstats/crashstats/static/jsonview/jsonview.custom.less
@@ -1,60 +1,27 @@
@import "@{root-path}/base/variables.less";

.jsonview {
.json-document {
background-color: @black;
color: @white;
color: @jsonview-prop;
font-family: monospace;
font-size: 1.1em;
white-space: pre-wrap;

.prop {
a,
a:hover,
a:visited,
.json-toggle,
.json-dict {
color: @jsonview-prop;
}
.num {
.json-literal {
color: @primary-light;
}
.null,
.bool {
color: @jsonview-bool;
}
.string {
.json-string {
color: @green;
white-space: pre-wrap;
}
.string.multiline {
display: inline-block;
vertical-align: text-top;
}
.collapser {
position: absolute;
left: -1em;
cursor: pointer;
}
.collapsible {
transition: height 1.2s;
transition: width 1.2s;
}
.collapsible.collapsed {
height: .8em;
width: 1em;
display: inline-block;
overflow: hidden;
margin: 0;
}
.collapsible.collapsed:before {
content: "…";
width: 1em;
margin-left: .2em;
}
.collapser.collapsed {
transform: rotate(0deg);
}
.q {
display: inline-block;
width: 0px;
color: transparent;
}
ul {
ol, ul {
list-style: none;
margin: 0 0 0 2em;
padding: 0;
Expand All @@ -64,7 +31,4 @@
position: relative;
}
}
h1 {
font-size: 1.2em;
}
}
5 changes: 3 additions & 2 deletions webapp/crashstats/crashstats/tests/test_views.py
Expand Up @@ -3,6 +3,7 @@
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

import copy
from io import StringIO
import json
import re
from unittest import mock
Expand Down Expand Up @@ -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("")})
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# the reason for first causing an exception to be raised is because
# the handler500 function is only called by django when an exception
Expand All @@ -223,7 +224,7 @@ def test_json(self):
views = __import__(par, globals(), locals(), [end], 0)
handler500 = getattr(views, end)

fake_request = RequestFactory().request(**{"wsgi.input": None})
fake_request = RequestFactory().request(**{"wsgi.input": StringIO("")})
# This is what the utils.json_view decorator sets on views
# that should eventually return JSON.
fake_request._json_view = True
Expand Down