[Patches] [ python-Patches-1755841 ] Patch for [ 735515 ] urllib2 should cache 301 redir

[SourceForge.net]

Patches item #1755841, was opened at 2007-07-17 21:32
Message generated for change (Comment added) made by montanaro
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1755841&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.6
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: O.R.Senthil Kumaran (orsenthil)
Assigned to: Nobody/Anonymous (nobody)
Summary: Patch for [ 735515 ] urllib2 should cache 301 redir

Initial Comment:
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.



----------------------------------------------------------------------

>Comment By: Skip Montanaro (montanaro)
Date: 2007-08-02 05:55

Message:
Logged In: YES 
user_id=44345
Originator: NO

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


----------------------------------------------------------------------

Comment By: O.R.Senthil Kumaran (orsenthil)
Date: 2007-08-01 23:17

Message:
Logged In: YES 
user_id=942711
Originator: YES

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


----------------------------------------------------------------------

Comment By: Skip Montanaro (montanaro)
Date: 2007-07-26 09:38

Message:
Logged In: YES 
user_id=44345
Originator: NO

Looks reasonable to me, however I wonder if you shouldn't purge cached
information periodically.  Does RFC 2616 provide any guidance?

Skip


----------------------------------------------------------------------

Comment By: O.R.Senthil Kumaran (orsenthil)
Date: 2007-07-23 22:56

Message:
Logged In: YES 
user_id=942711
Originator: YES

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

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1755841&group_id=5470