Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7453 closed (wontfix)

HttpResponse uses parameter "status" to set "status_code" which is mildly confusing.

Reported by: Simon Greenhill Owned by: telenieko
Component: HTTP handling Version: master
Severity: Keywords: HttpResponse status_code
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

HttpResponse uses the parameter status to set the attribute status_code. This is inconsistent and confusing. I suspect it's caused the problems found on #7046, and we should consolidate the usage to either status or status_code.

Attachments (2)

7453.diff (1.9 KB) - added by telenieko 6 years ago.
Fix.
7453.backwards.diff (2.3 KB) - added by telenieko 6 years ago.
Non silent backwards compatible version.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by telenieko

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to telenieko
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by Simon Greenhill

...and the same confusion in #7234.

Changed 6 years ago by telenieko

Fix.

comment:3 Changed 6 years ago by telenieko

  • Has patch set

Attached patch fixes this, passes tests and updates docs accordingly.

comment:4 follow-up: Changed 6 years ago by russellm

  • milestone set to 1.0 alpha
  • Triage Stage changed from Accepted to Design decision needed

Consistency is a good argument for this change, but this would be a backwards incompatible change (probably not a huge impact change, but a change nonetheless). If we're going to make the change, we need to do it pre v1.0. Marking for a v1.0 decision.

comment:5 in reply to: ↑ 4 Changed 6 years ago by telenieko

Replying to russellm:

Consistency is a good argument for this change, but this would be a backwards incompatible change (probably not a huge impact change, but a change nonetheless). If we're going to make the change, we need to do it pre v1.0. Marking for a v1.0 decision.

We can accept both status and status_code and raise a DeprecationWarning when status is set, it's just 3 more lines of code ;)
What do you think?

comment:6 Changed 6 years ago by telenieko

That's a backwards compatible version, which raises a PendingDeprecationWarning, for some reason my python doesn't show them. But it gets raised ;)

Changed 6 years ago by telenieko

Non silent backwards compatible version.

comment:7 Changed 6 years ago by garcia_marc

  • milestone changed from 1.0 alpha to 1.0 beta

According to ticket organization defined in http://code.djangoproject.com/wiki/VersionOneRoadmap#how-you-can-help 1.0 alpha tickets should be just features in the Must have (http://code.djangoproject.com/wiki/VersionOneRoadmap#must-have-features) list.

Changing to 1.0 beta for marking as v1.0 decision.

comment:8 Changed 6 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from assigned to closed

Not worth it. You set status once when you initialise the response and that's it. There's not really any call for normal code to be poking at status_code, so it could be called flibble and people shouldn't care. It's certainly not worth adding so many lines of code just to avoid the backwards incompatible thing, but I think it's really not worth doing at all. We've had that parameter there for a while now and people are using it. Let's not break their code for no reason.

comment:9 Changed 3 years ago by jacob

  • milestone 1.0 beta deleted

Milestone 1.0 beta deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.