Opened 14 months ago

Closed 10 months ago

Last modified 10 months ago

#32259 closed New feature (wontfix)

Modernize request attribute names

Reported by: Adam Johnson Owned by: Adam Johnson
Component: HTTP handling Version: dev
Severity: Normal Keywords:
Cc: אורי, Carlton Gibson Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As discussed on the mailing list (back in May):

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 on 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. ( https://www.php.net/manual/en/reserved.variables.get.php ). It stands out as unpythonic, since these are instance variables and not module-level constants (as per PEP8 https://www.python.org/dev/peps/pep-0008/#constants ).

I therefore propose these renames:

  • request.GET -> request.query_params (to match Django Rest Framework - see below)
  • request.POST -> request.form_data
  • request.FILES -> request.files
  • request.COOKIES -> request.cookies
  • request.META -> request.meta

The biggest concern on the mailing list was churn. Therefore we won't deprecate the old names, but will document them as historical aliases.

Change History (10)

comment:1 Changed 14 months ago by Adam Johnson

Owner: changed from nobody to Adam Johnson

comment:2 Changed 14 months ago by אורי

What will happen to existing code who will use request.GET or request.META?

I found out that we are also using request.LANGUAGE_CODE.

comment:3 Changed 14 months ago by אורי

Cc: אורי added

comment:4 Changed 14 months ago by Carlton Gibson

Triage Stage: UnreviewedAccepted

I think the consensus was to have this.

Leaving the old API in place, documented just once in the request docs, is sufficient to avoid a forced rewrite. So +1.

(back in May)

Funnily enough this crossed my mind just yesterday too. :)

comment:5 Changed 14 months ago by Carlton Gibson

Type: Cleanup/optimizationNew feature

I'm going to call this a New Feature. It will need release notes.

comment:6 Changed 14 months ago by Adam Johnson

I found out that we are also using request.LANGUAGE_CODE.

Thanks. I'm not sure if this one is such high value to change, it's capitalized to indicate that it overrides the setting.

I'm going to call this a New Feature. It will need release notes.

Fair enough!

comment:7 Changed 14 months ago by Adam Johnson

Has patch: set

comment:8 Changed 14 months ago by Adam Johnson

Summary: Rename request attributesModernize request attribute names

comment:9 Changed 10 months ago by Mariusz Felisiak

Cc: Carlton Gibson added
Resolution: wontfix
Status: assignedclosed

Adam, thanks for creating a ticket. However, this change is really disruptive and it will affect everyone. In the case of such changes, it's crucial to have a strong consensus and clear significant benefits. I don't see a strong consensus on the mailing list and benefits are not convincing.

comment:10 Changed 10 months ago by Mariusz Felisiak

Triage Stage: AcceptedUnreviewed
Note: See TracTickets for help on using tickets.
Back to Top