Code

Opened 4 years ago

Closed 3 years ago

#14903 closed Cleanup/optimization (fixed)

wsgiref usage

Reported by: maxbublis Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: aymeric.augustin@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

Currently Django uses copy-pasted wsgiref as its development server. Since wsgiref is a part of standard Python distribution since 2.5, there is no need to distribute wsgiref implementation with Django.
The patch is not breaking current test suits. It should be applied only after official announcation of Python 2.4 support discontinue in Django. As I know, Python 2.4 support will not be dropped in coming 1.3 release, so patching should be defered.
Also this patch would possibly lead to smoother Python 3.x transition in future.

Attachments (2)

wsgiref_usage.diff (23.7 KB) - added by maxbublis 4 years ago.
Patch replacing current development server with wsgiref
14903.patch (21.7 KB) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by maxbublis

Patch replacing current development server with wsgiref

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Accepted for a future release (whenever we decide to drop Python 2.4 support; Django 1.4 is likely).

comment:2 Changed 4 years ago by russellm

A quite note for potential reviewers: although the original may have been copy-pasted, It's entirely likely that we have made tweaks to address bugs along the way. Anyone reviewing this should do an audit of the blame logs and find all the places and times that we've made modifications, and make sure that those fixes either exist upstream.

comment:3 Changed 3 years ago by jaddison

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:4 Changed 3 years ago by aaugustin

  • Cc aymeric.augustin@… added

I re-did the job from scratch in order to check if I would end up with the same patch. I mostly did :)

Interesting differences with the original patch are:

  • I updated the module's docstring
  • I kept the customized error_status - the value in the stdlib is a bit whacky
  • I kept the customized get_environ() method of WSGIRequestHandler - it's fixing a bug
  • I removed the handle() method of WSGIRequestHandler - it's not changed

I compared every line I removed with the source of Python 2.5.5.

I checked the commit history for django/core/servers/basehttp.py back to r174, where the file was first committed. All changes are preserved by the patch, except the following:

  • r6780 is lost. However, #6063 was re-opened and the root cause was fixed at r6895, making the change in r6780 unecessary. So it is fine.
  • r6634 is lost. However, the definition of format_date_time() in wsgiref.handlers is correct. #5816 was actually introduced by a incorrect fix in r5712. I think wsgiref.handlers was fixed independently after Django forked it.
  • r5511 reverts r5483.
  • r5091 is lost. However, the goal was a "slight performance improvement". This does not seem a sufficient reason to keep a forked version of a module of the standard library.
  • r3530 was lost by the previous patch, which would have caused a regression on #2494. I fixed that in my patch.
  • r3113 was lost by the previous patch, which was probably innocuous. I fixed that in my patch.
  • r2809 is the customized error_status mentioned above.

All tests pass.

Not marking this as RFC since I modified the patch, but I think it's pretty close.

Changed 3 years ago by aaugustin

comment:5 Changed 3 years ago by aaugustin

Just updated the patch to fix a re-indentation issue.

comment:6 Changed 3 years ago by jezdez

  • Component changed from Core (Other) to HTTP handling
  • Easy pickings unset
  • Triage Stage changed from Accepted to Ready for checkin

Wow, what a patch. I can confirm that all tests pass for me, so unless anyone else has problems with this, it's ready for commit.

comment:7 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [16288]:

Fixed #14903 -- Removed much of the code in django.core.servers.basehttp that was previously copy-pasted from the wsgiref module which was added to Python 2.5. Many thanks to maxbublis and aaugustin.

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.