Technical Board: vote on Ticket #32259, "Modernize request attribute names"

In May 2020 I wrote to django-developers proposing a rename of some core attributes on HttpRequest:

As a reminder the proposal is this:

request.GET and request.POST are misleadingly named:

  • GET contains the URL parameters and is therefore available whatever the request method. This often confuses beginners and “returners” alike.
  • POST contains form data for POST requests, but not other kinds of data from POST requests. It can confuse users who are posting JSON or other formats.

Additionally both names can lead users to think e.g. if request.GET: means if this is a GET request, which is not true.

I believe the CAPITALIZED naming style was inherited from PHP’s global variables $_GET, $_POST, $_FILES etc. ( PHP docs ). It stands out as unpythonic, since these are instance variables and not module-level constants (as per PEP8 ).

I therefore propose these renames:

  • request.GETrequest.query_params (to match Django Rest Framework - discussed later in mail)
  • request.POSTrequest.form_data
  • request.FILESrequest.files
  • request.COOKIESrequest.cookies
  • request.METArequest.meta

I later opened Ticket #32259, with a PR. One fellow accepted the ticket, and the other later closed it as wontfix for being too disruptive.

I can see the problem with the churn, but I still believe the change is worth it. When I’ve since mentioned it to other Django developers they have been in favour.

I think we can control the churn. The old names will be left in place with no deprecation warnings, the release notes will have a section covering the change, and you can bet I’ll write a blog post. Also my recent project django-upgrade will have a “fixer” to automatically migrate projects to the new names.

So as per the DEP 10 voting process I am calling on the technical board to vote on the proposal:

Shall the Django project accept and begin implementation of Ticket #32259?

Asking for votes from the other members of the technical board: @apollo13 @andrewgodwin @charettes @orf

The voting period of 1 week means this will close on the 2nd November.

For the record my vote is “accept”.

2 Likes

Technically according to DEP 10 each vote MUST be one of the following: “+1”, “0”, or “-1”.

Right yes I got mixed up with the outcome - for clarity my vote is +1

I do feel that straight to a TB vote is a little quick here.

TB are able to make their minds up sure, but it would be good to go over the case and see if we can advance the consensus first.

I’d also like us to hash out any migration plan before calling it to a vote — otherwise it’s a bit like the Brexit vote: “Say yes, but it’s not clear exactly what to — you’ll find that out later”.

However, that’s procedural. If you’re sure on an immediate vote, DEP 10 allows that.

Whilst I originally accepted the ticket, I agreed with Mariusz’ close. (We discussed it, and agreed at the time.)

They were a significant number of voices on the discussion raising the concerns about churn. The proposed API is nicer, but on its own it’s not clear that’s enough to override the API Stability Policy, and we’ve had a lot of push back many times at the end of deprecation periods as the old API stops working. That’s not to consider the history of documentation that’s obsoleted by this kind of thing. I think we need to tread carefully (and so again discuss rather than rush to a vote,)

I like the proposal, but think we need more to justify it than just nice API. (It’s a lot of churn.) Specifically I was hoping that we could bundle it up with improvements along the lines of #21442 on Content Negotiation (not necessarily that exactly, but along the lines) in order to be able to say, There’s a new API, and new features that pay for it. If we had more to say here I’d be all for it.

Perhaps bike-shedding here, I also have one concern over POST to form_data, since GET forms are a thing, and folks will be Where’s my form data? — but that’s surely addressable. (Particularly if body_data was handled with regard to content type.)

Sorry, yes. I was just a little bit frustrated and wasn’t sure what the next best course of action would be.

This is fair.

My proposal is to keep the old aliases with no plan of deprecation and no warnings. They’re not much code, and can stick around like HttpResponse.__getitem__, Meta.index_together, etc. Sorry if that’s not clear.

Whilst the API stability policy does say we aim to keep to “only one way”, I think the churn concerns should override this, as with the aforementioned never-deprecated API’s.

I see the renaming of these attributes as orthogonal to extending any capabilities in HttpRequest or similar. The renaming is more in the camp of HttpResponse.headers - making code clearer.

I think then this is where the core of the difference in judgements lies.

Here’s a filter to just the docs changes from your original PR — just the .txt files.

Throughout the ecosystem, everything I’ve ever read, everywhere, no longer corresponds to what I find in the docs. Even if we keep the old API alive as aliases, I think we’re seriously understating the amount of disruption that’s going to cause.

As folks deeply vested in Django, we hear a suggestion like this, and think Ooh, yes, that would be good, but most of our users aren’t so closely involved.

To then say we’ll just change the documented API without further benefit is (if you have your finger on the merge button) slightly eye-watering. It’s nicer yes. It’s hard to see whether it merits the cost in the wider user-space though. We’ve done massively well keeping Django stable whilst progressing it. Stability is arguably our USP — that’s what folks rely on.

If, in contrast, though, we’re able to offer a new set of capacities on HttpRequest, then there’s a story to tell. There’s this new API, and look what it gives you. (If it’s just aliases, wouldn’t a ModerniseRequestMiddleware do in the meantime?) OK, disruption, but it’s worth it.

(Or so goes the line of reasoning.)

I’ll let others comment, since you’ve invoked the powers :grinning_face_with_smiling_eyes:

2 Likes

That is a good question and I certainly understand the frustration. I think what you are proposing is generally worth following, so even if the vote goes to no action/veto I hope you/we can continue pushing it towards the finish line.

I think I am -1 on the proposal as it currently stands (please note that this is not yet my vote, I still have a few days ;)). It is not necessarily due to the churn that Carlton mentioned but on a more technical level.

While I have no issues at all with the changes for GET/META/FILES/COOKIES I am a bit worried (still) about POST and renaming it to form_data. I’d like to know if we still think content negotiation is something worth pursuing (I think it is). If the answer to the previous question is “yes” then I’d like to ask how data from the content negotiation will end up on the request object. Following DRF here it would most likely be request.data. Which brings me to the next question: Will we deprecate form_data after we introduced data? Wouldn’t it make more sense to call it data right now even if it is just for form data? So all in all I think that a disruptive change like this should consider further changes like content negotiation.

Does that make sense?

Cheers,
Florian

It’s still desired, e.g. #32646 (add request.json() shortcut) – Django recently, which referenced this topic.

My vote is currently 0 as I don’t think there’s been quite enough community discussion on this yet, even though I agree with the overarching goal here. Can we try to restart the discussion and see if a consensus can be reached before advancing to a TB vote?

Just so it is official I’ll add my vote here as “0” since I think we should persue the plan after the existing questions are resolved and 0 rather than -1 will hopefully ensure that we can put this back to the technical board (if needed) with no delays.