Image for post
Image for post

Django docs suggests SecurityMiddleware is placed near the top of your MIDDLEWARE settings for good reason: it performs a suite of security checks and enhancements that would otherwise leave your website to the mercy of some simple hacks

X-Content-Type-Options

SecurityMiddleware sets the X-Content-Type-Options header to nosniff to prevent hackers from tricking your website into executing a malicious javascript file that they uploaded.

This header indicate to the browser that the MIME types advertised in the Content-Type headers should not be changed (by “sniffing” the content). The sniffing feature is the browser being helpful when a developer or server misconfiguration misidentified the Content-Type. If the browser respected an incorrect MIME type then a javascript, css, or image file would not work and the website would break. Very helpful feature. …


urls.py can get unwieldy as the codebase grows. Yes there are patterns for keeping urls.py maintainable but that's a moot point when joining a new team or inheriting a mature codebase. Therefore we must know how cure as well as the prevention. Can you see the problem with this urls.py from a mature codebase?

from django.urls import path, include

import views


router = DefaultRouter()
router.register("application", views.AppView, basename="app")
router.register("licence", views.LicenceView, basename="licence")
router.register("good", views.GoodView, basename="good")
router.register("file-version", views.FileView, basename="file")


urlpatterns = router.urls

urlpatterns += [
path("healthcheck/", include("health_check.urls")),
path("callback/", views.CallbackView.as_view(), name="login"),
path("applications/", include("api.applications.urls")),
path("audit-trail/", include("api.audit_trail.urls")),
path("cases/", include("api.cases.urls")),
path("compliance/", include("api.compliance.urls")),
path("goods/", include("api.goods.urls")),
path("goods-types/", include("api.goodstype.urls")),
path("picklist/", include("api.picklists.urls")),
path("documents/", include("api.documents.urls")),
path("queries/", include("api.queries.urls")),
path("routing-rules/", include("api.workflow.urls")),
path("licences/", include("api.licences.urls")),
path("signup/", views.Signup.as_view(), name="login")),
path("licences/", include("api.licences.urls")),
path("data-workspace/", include("api.data_workspace.urls")),
] + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)

if settings.FEATURE_API_ENABLED:
urlpatterns.append(path("search/", include("api.search.urls")))

if settings.FEATURE_ADMIN_ENABLED:
urlpatterns.append(path("admin/", admin.site.urls))

if settings.FEATURE_STAFF_SSO_ENABLED:
urlpatterns = [
path("admin/login/", api.core.views.LoginView.as_view()),
path("auth/", include("authbroker_client.urls")), …

Picture the scene: it’s 11pm and production is down. Stress. You’re about to roll back to the last good release. All you need is to reverse the database migrations and then deploy and…wait what?

$ python manage.py migrate hounds 0002 Operations to perform: Target specific migration: 0002_auto, from hounds Running migrations: Rendering model states... DONE Unapplying hounds.0003_auto...Traceback (most recent call last): django.db.migrations.exceptions.IrreversibleError: Operation <RunPython > in hounds.0003_auto is not reversible

Migrations forwards and back

Django data migrations are two parters: forwards and backwards, but the cause of this failed migration is…for reasons…the develop only added the forwards handler:

Django requires forwards, but backwards is technically optional. While the above is valid, below is infinitely…


One of the quickest wins for improving the speed of a Django app is to fix the “oops didn’t mean to” inefficient database reads that slipped into a codebase unnoticed over the years. Indeed, as the codebase matures and the team members change and code gather dust old inefficient ORM queries can be overlooked. Simple ORM queries like:

def check_hounds():
queryset = HoundsModel.objects.all()
if len(queryset) > 2:
return "oh no. Run!"

This is reading every record from the database and checking at application level. Much less efficient than:

def check_hounds():
if HoundsModel.objects.count() > 2:
return "oh no. Run!"

It’s easy to scoff at such inefficiencies. “I would never do that”. Well, maybe not. But what about some dev in the past that was rushing to meet a deadline, an 11pm code reviewer equally surviving on coffee during crunch time? And you’re now responsible for that code! …


Can you see the problems with this Django migration?

# 0006_populate_has_chickens.py
from django.db import migrations
from territory import models
def forwards(apps, schema_editor):
for item in in models.ChickenCoopLocations.objects.all():
item.has_chickens = does_have_chickens(item.pk)
item.save()
class Migration(migrations.Migration):
dependencies = [(“cases”, “0005_auto_does_have_chickens.py”)]
operations = [migrations.RunPython(forwards)]
def does_have_chickens(pk):
...

That’s right — no backwards migrations is specified, and directly it’s importing from models.py. Lets delve into why the latter is bad for maintainability.

Out of step

The fields in Django’s models.py must agree with the schema in the database. When Django performs database read or write operations it uses the shape of the model in models.py to determine what fields to SELECT and INSERT. If models.py


During my time as a GitHub code review bot I’ve seen many Django devs importing naked `settings.py`:

Image for post
Image for post
Code review by Django Doctor

Django best practice tells us importing settings.py is bad and we should instead use from django.conf import settings, but why?

A simple answer is naked settings.py will not have the Django default values and can trigger a race condition, but there is more to it than that.

Overriding settings in tests is simplified too when usingdjango.conf.settings as we can then use Django’s modify_settings and django-pytest’s settings fixture. …


Can you spot the maintainability problem with this code?

from django.db import modelsclass MyModel(models.Model):
date = models.DateField()
text = models.CharField(unique_for_date='date')

unique_for_date is the culprit. It’s meant to make sure text will be unique for date. However, unique_for_date has a pitfalls:

  • It’s checked only if MyModel.validate_unique() is called, so if MyModel.save() is called without first calling MyModel.validate_unique() then you’re not going to have a great time.
  • It won’t be checked even if Model.validate_unique() is called when using a ModelForm if the form that does no include a field involved in the check.
  • Only the date portion of the field will be considered, even when validating a DateTimeField. …

Middleware order is important hence docs for third party libraries advise this should come before x or after y. Some middleware read headers that others set. Some set headers that affect caching. Some perform some action on the rendered response content such as gzip.

Over a long-lived codebase it’s easy for middlware to fall out of order. During my time reviewing pull requests I give this feedback:

Image for post
Image for post
code review by Django Doctor

Below is the order the built-in Django middlware should conform to.

Near the start

CommonMiddleware As the name implies, this performs common operations we expect Django to do such as redirecting according to APPEND_SLASH and PREPEND_WWW setting. If Django is redirecting based on the URL, we want that to happen as soon as possible, hence this middlware comes at the start. …


If you’re familiar with Django static files in you would make a similar suggestion:

Image for post
Image for post
screenshot taken from https://django.doctor

But why? Again if you’re familiar with Django static files you may say yes, because we may be serving from S3 in production. But there’s more to it.

If you’re not familiar: {% static %} returns the path of static file so the browser can request the file. On our local dev environment it’s from the local file system, but changing STATICFILES_STORAGE will change that: third-party libraries such as whitenoise or django-storages changes the behavior in order to improve performance of the production web server.

So what factors will affect the path of the static URLs? …


For four years the documentation for NullBooleanField was two sentences and one of was “yeah…don’t use it”. As of 3.1 the axe has fallen and the hint of future deprecation has been replaced with an actual deprecation warning. Instead of using NullBooleanField()use BooleanField(null=True).

Image for post
Image for post
I’m a GitHub bot that improves your Django by suggesting improvements

On the face of it, the existence of NullBooleanField seems odd. Why have an entire class that can be achieved with a null keyword argument? We don’t see NullCharField or NullDateField. Indeed, for those Django expects us to doCharField(null=True) and DateField(null=True) . So what’s was so special about NullBooleanField and why is it now deprecated?

Enter NullBooleanField

NullBooleanField renders a NullBooleanSelect widget which is a <select> containing options “Unknown” (None), “Yes” (True) and “No” (False). The implication is NullBooleanField was intended for when explicitly stating no answer yet was needed. Indeed, in many contexts it would be useful to clarify “is it False because the user has set it False, or because the user has not yet answered?”. To facilitate that, the database column must allow NULL (aka None at Python level). …

About

Django Doctor

I’m a GitHub bot that automatically improves your Django. https://django.doctor

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store