Opened 16 years ago

Closed 16 years ago

#6165 closed (wontfix)

request.get_host() should be request.host

Reported by: eratothene Owned by: nobody
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Looking the HttpRequest class, I see all properties has plain name like: request.method, request.user, request.path. No methods that have get_smth() name, hence for holding have convention I propose to rename request.get_host() with request.host. As request.get_host() is not documented, there is no backward compatibilty problem, but the depreciated get_host() can remain for some time.

Also request.get_host() is not documented yet. I can do this as well.

Let's agree on idea and I will make patch.

Attachments (2)

patch.diff (1.3 KB ) - added by eratothene 16 years ago.
Incomplete patch
patch.2.diff (862 bytes ) - added by eratothene 16 years ago.
Incomplete patch

Download all attachments as: .zip

Change History (10)

by eratothene, 16 years ago

Attachment: patch.diff added

Incomplete patch

by eratothene, 16 years ago

Attachment: patch.2.diff added

Incomplete patch

comment:1 by eratothene, 16 years ago

I have submitted a patch, it is definitely not complete, but it shows an idea. If you like it, I will make a full version.

comment:2 by Chris Beaven, 16 years ago

Triage Stage: UnreviewedDesign decision needed

Ticket #6119 is about documenting request.get_host()

in reply to:  description ; comment:3 by Chris Beaven, 16 years ago

Replying to eratothene:

Looking the HttpRequest class, I see all properties has plain name like: request.method, request.user, request.path. No methods that have get_smth() name, hence for holding have convention I propose to rename request.get_host() with request.host.

I'm not sure it's worth the code churn. And we do have a get_ method already: get_full_path()

Then again, it does have a similar "feel" to request.path, so I'll leave open as a design decision. Perhaps you can bring this up in the django-dev group?

comment:4 by eratothene, 16 years ago

Digging futher I have found out that request.get_host() is called at least two times per each reques: in common middleware, in base handler (fix)location_header). Also number of get_host() calls grows if contrib.sites is used with RequestSite, which is also common.
As it is clear stated in the doc/request_response.txt: "HttpRequest objects. All attributes except session should be considered read-only". I propose to cache the host value as soon as it gets evaluted, as it takes quite a lot of steps to determine the host name.

in reply to:  3 ; comment:5 by eratothene, 16 years ago

Replying to SmileyChris:

Replying to eratothene:

Looking the HttpRequest class, I see all properties has plain name like: request.method, request.user, request.path. No methods that have get_smth() name, hence for holding have convention I propose to rename request.get_host() with request.host.

I'm not sure it's worth the code churn. And we do have a get_ method already: get_full_path()

Then again, it does have a similar "feel" to request.path, so I'll leave open as a design decision. Perhaps you can bring this up in the django-dev group?

I am looking at current revision of django/http/init.py is 6895, request.get_full_path() returns empty string nothing else. Sounds odd to me. Is it scheduled for removal?

in reply to:  5 comment:6 by Chris Beaven, 16 years ago

Replying to eratothene:

I am looking at current revision of django/http/init.py is 6895, request.get_full_path() returns empty string nothing else. Sounds odd to me. Is it scheduled for removal?

Odd indeed. Go ask the group.

in reply to:  5 comment:7 by arien, 16 years ago

Replying to eratothene:

I am looking at current revision of django/http/init.py is 6895, request.get_full_path() returns empty string nothing else. Sounds odd to me. Is it scheduled for removal?

No, request.get_full_path() is overridden in ModPythonRequest and WSGIRequest. A comment stating that might be nice though. :-)

comment:8 by Malcolm Tredinnick, 16 years ago

Resolution: wontfix
Status: newclosed

I don't think this is worth changing at the moment. There is a fair bit of code that is either known to be using it or potentially using it and adding a synonym "host" property doesn't really add a lot of value. There's no real functionality improvement.

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