classification
Title: Patch for [ 735515 ] urllib2 should cache 301 redir
Type: feature request Stage: patch review
Components: Library (Lib) Versions: Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: jjlee, orsenthil, vila (3)
Priority: normal Keywords patch

Created on 2007-07-18 02:32 by orsenthil, last changed 2009-08-21 01:18 by orsenthil.

Files
File name Uploaded Description Edit Remove
urllib2-301-cache.patch orsenthil, 2007-08-05 20:41 patch to add urllib2 301 cache redirection. version 3
test_urllib2-cache.patch orsenthil, 2007-08-05 20:42 test_urllib2 with 301 cache tests included
liburllib2.patch orsenthil, 2007-08-05 20:43 Patch for Documentation
Messages (9)
msg52902 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-07-18 02:32
Tracker item python.org/sf/735515 mentions about urllib2 to cache 301 redirections.
Attached patch tries to implement the same.

Comments on Version 1 of patch:
a) Initializes a dictionary to store the redirection.
b) If req already in cache, return the previous Request object.
c) Otherwise handle the same as 302 and store the Request object.
d) Checks for loop errors in 301.

Just noticed, that it I missed max-redirect checks.
Please comment on this patch, with next version I shall add the max redirect check.

msg52903 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-07-24 03:56
I have made some changes to patch. Made it use just the self.cache to cache and follow the same logic for redirect and loop as 302 would follow. I thought this is ok. Did a basic testing to verify if it caught 301 redirect, yes it does.

File Added: urllib2-cache-301.patch
msg52904 - (view) Author: Skip Montanaro (skip.montanaro) Date: 2007-07-26 14:38
Looks reasonable to me, however I wonder if you shouldn't purge cached information periodically.  Does RFC 2616 provide any guidance?

Skip
msg52905 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-08-02 04:17
Hi Skip,
I have implemented Cache redirection for 301 errors only which will be Permanant redirection. Expiry header is not expected for 301 direction, so we might not purge it.
RFC 2616 states that if cache is implemented for 302 redirects ( temporary redirects) than Expiry header should be handled and cache purged.

For our change it is not required.

Thanks,
Senthil
msg52906 - (view) Author: Skip Montanaro (skip.montanaro) Date: 2007-08-02 10:55
How much extra work would it be to handle 302 redirects and the Expiry header?  (I don't want to hold you up from the rest of your SoC tasks.  Time's running out.)

Note also that you should also include a patch to the urllib2 documentation and, if possible, one or more test cases to exercise the new code.

Skip
msg52907 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-08-05 20:41
File Added: urllib2-301-cache.patch
msg52908 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-08-05 20:42
File Added: test_urllib2-cache.patch
msg52909 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-08-05 20:43
File Added: liburllib2.patch
msg52910 - (view) Author: Senthil Kumaran (orsenthil) Date: 2007-08-05 20:59
Hi Skip,
Added the updated patch and also added patches to test and documentation also.

Did some investigation on Cache redirection and expires for 302. IMHO, it should be handled as a separate feature request (based on the need), as we will have to handle the Cache-Control header values in addition to expires header. This would amount for good amount of change w.r.t the HTTPRedirectHandler itself.
[http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html section 14.9]

As for the current Tracker item, please verify and if found suitable checkin to the Python trunk.

Thanks,
Senthil


History
Date User Action Args
2009-08-21 01:18:58orsenthilsetassignee: orsenthil
2009-04-28 00:49:41skip.montanarosetnosy: - skip.montanaro
2009-04-27 22:41:38ajaksu2linkissue735515 dependencies
2009-04-27 22:41:20ajaksu2setnosy: + jjlee

type: feature request
versions: + Python 2.7, - Python 2.6
stage: patch review
2008-01-05 13:07:03vilasetnosy: + vila
2007-07-18 02:32:18orsenthilcreate