Fixed #24137 -- Use default reason phrases from HTTP standard.

[jdufresne]

No description provided.

[collinanderson]

Hah. I've always used the all caps as a hint that it's django serving the request :).

[jdufresne]

Hah. I've always used the all caps as a hint that it's django serving the request :).

That seems like an opportunity for a malicious user to sniff that a page was served by a Django application server.

[berkerpeksag]

Is there a specific reason to duplicate this dict in Django? httplib.responses already covers many of them. On Python 3, http.client.responses has even more complete list.

[jdufresne]

Is there a specific reason to duplicate this dict in Django? httplib.responses already covers many of them. On Python 3, http.client.responses has even more complete list.

I'm happy to alter the code to use the values from Python stdlib. Should this still be assigned to the variable REASON_PHRASES for backwards compatibility or is that considered an internal detail?

[MarkusH]

There is an import in django.core.handlers.wsgi that references REASON_PHRASES:

# For backwards compatibility -- lots of code uses this in the wild!
from django.http.response import REASON_PHRASES as STATUS_CODE_TEXT

I'd like to keep that around for a while and add a deprecation.

Another thing I'd like to see, is a note in the release notes under "Backwards incompatible changes" that the all-uppercase property for https://docs.djangoproject.com/en/dev/ref/request-response/#django.http.HttpResponse.reason_phrase (and others on that page) changes to the defaults provided through the Python stdlib.

[jdufresne]

I have made the suggested changes:

• Reason phrases are pulled from Python stdlib; http.client.responses on Python 3 and httplib.responses on Python 2.
• REASON_PHRASES and STATUS_CODE_TEXT are now an alias for these values.
• REASON_PHRASES and STATUS_CODE_TEXT are both deprecated for 2.0.
• Added documentation to the release notes and reason_phrase attribute documentation.

Some thoughts from me:

• Is my approach for deprecating the REASON_PHRASES dictionary good/correct? I could not find other examples where a dictionary was deprecated. I want to signal a warning on access of a dictionary item.
• Is there special rst syntax to link to Python's documentation from Django documentation?
• Please feel free to make strong suggestions with regards to my documentation changes. I don't consider myself a strong technical writer.
• I realize we're in a feature freeze; I may need to move some of the documentation and deprecation versions. That is no problem.

[claudep]

I think that we could make use of a six moved import: six.http_client (https://pythonhosted.org/six/#module-six.moves)

[jdufresne]

TODO:

• Move DeprecatedDict to own commit with tests
• Raise concern over missing phrases on django-developers

[jdufresne]

The following are not in the stdlib (checked Python 3.4): ...

Found this upstream bug http://bugs.python.org/issue21793 and changeset https://hg.python.org/cpython/rev/edf669b13482 which resolves this concern. However, this does not help users of Python < 3.5.

[claudep]

We can test the Python version and complete the dict for Python < 3.5

[aaugustin]

Or we can admit that reason phrases are purely cosmetic and leave it there :-)

[jdufresne]

I have separated out the commits and added tests.

We can test the Python version and complete the dict for Python < 3.5

I have done this in the most recent commit.

Or we can admit that reason phrases are purely cosmetic and leave it there :-)

I'm happy to remove the version check and leave the dictionaries as is. I agree, this shouldn't present a real problem as 1. These status codes are either later additions or non-standard 2. These phrases are intended only for humans.

I'll do whatever is agreed upon here as the best course. Removing the version check will simplify the code any remove any future maintenance burden as versions go unsupported.

[collinanderson]

I think I agree with Aymeric. I'd (personally) prefer simpler code: use whatever python provides and you can set the reason message by hand if you want more than that.

REASON_PHRASES and STATUS_CODE_TEXT are both undocumented, so I don't think it makes sense to bend-over-backward just to raise a warning. Mentioning the changes in the release notes should be good enough it seems to me.

If we were clever in our importing (like "from django.utils.six.moves.http_client import responses as REASON_PHRASES" and then keep using REASON_PHRASES) that could make things smoother for people.

[jdufresne]

I think I agree with Aymeric. I'd (personally) prefer simpler code: use whatever python provides and you can set the reason message by hand if you want more than that.

Thanks.

@claudep How do you feel about these responses?

At this point is there consensus? Is it worth still raising this issue on django-developers? Or should I tear out the version check?

---

REASON_PHRASES and STATUS_CODE_TEXT are both undocumented, so I don't think it makes sense to bend-over-backward just to raise a warning. Mentioning the changes in the release notes should be good enough it seems to me.

Are you suggesting I drop the deprecation path? Documenting the change is sufficient?

If we were clever in our importing (like "from django.utils.six.moves.http_client import responses as REASON_PHRASES" and then keep using REASON_PHRASES) that could make things smoother for people.

My opinion, either we support the REASON_PHRASES attribute or we don't. If we do, then what you say makes sense and it should be documented. If we don't we should either deprecate it with a deprecation path or remove it completely.

[collinanderson]

Are you suggesting I drop the deprecation path? Documenting the change is sufficient?

Yes, we don't need to have deprecation paths for undocumented features, unless they're really popular.

[jdufresne]

Yes, we don't need to have deprecation paths for undocumented features, unless they're really popular.

OK. I don't have a problem with this. But, this is in contrast to the comment above from @MarkusH

I'd like to keep that around for a while and add a deprecation.

I'm OK going either way on this, I just need some consensus so I know what to implement.

[jdufresne]

I have updated my PR and incorporated feedback. Some feedback is conflicting, but I chose what seems most appropriate and preferred by most people.

• No longer introduce a deprecation path as REASON_PHRASES and STATUS_CODE_TEXT were always internal details and not part of the documented API. Now, they are simply removed with a note in the release documentation.
• As reason phrases are simply for aesthetics, don't try to be clever with Python versions. Just use what is available in the stdlib.
• I have rebased on latest master branch and have resolved merge conflicts.

Any additional feedback is welcome. Thanks.

[jdufresne]

And then keep using REASON_PHRASES below. That will give a slight shim to anyone who imports REASON_PHRASES.

But what's the point?

IMHO using import as should generally be avoided unless there is a good reason to use it. I'd prefer not to add this here. You said

REASON_PHRASES and STATUS_CODE_TEXT are both undocumented, so I don't think it makes sense to bend-over-backward just to raise a warning. Mentioning the changes in the release notes should be good enough it seems to me.

and

Yes, we don't need to have deprecation paths for undocumented features, unless they're really popular.

These are both internal, undocumented module "constants" that simply shadow a stdlib constant. I see no hard evidence that these are popular. So I've taken the approach of simply mentioning them in the release notes.

I feel adding this shim only adds noise to the code -- makes reading and searching more tedious -- without solving a real problem.

If adding import as is the only way this will be merged then I will reluctantly add it.

[collinanderson]

I see no hard evidence that these are popular. So I've taken the approach of simply mentioning them in the release notes.

Good point. lgtm.

[timgraham]

This documentation is in the wrong section -- these are features that have gone through a deprecation cycle. The section you want is "Backwards incompatible changes"

[jdufresne]

@timgraham All feedback incorporated in the latest version.

Did you try using intersphinx for the Python 3 link?

This worked great! Thanks for the tip.

Is it worth adding a 'python2': ('https://docs.python.org/2/', None), to the intersphinx_mapping?

[berkerpeksag]

Could you please rebase your branch on top of master?

[jdufresne]

Could you please rebase your branch on top of master?

Done. Thanks.

[collinanderson]

buildbot, test this please

[timgraham]

Probably not worth adding Python 2 to intersphinx at this point. merged in 24b2bc6. Thanks Jon and all reviewers!