[MERGE] HTTP redirection

[Vincent Ladeuil]

>>>>> "aaron" == Aaron Bentley <aaron.bentley at utoronto.ca> writes:

<snip/>

    >> >> And my previous implementation was changing the way
    >> >> BzrDirMetaFormat1 behave (to avoid imposing the redirection
    >> >> handling to foreign branches).
    >> 
    aaron> It would be wrong to make the change for
    aaron> BzrDirMetaFormat1 only.  That would have introduced a
    aaron> regresion for BzrDir4,5&6.
    >> 
    >> No.
    >> 
    >> The previous implementation leaved the BzrDir4,5&6 handle
    >> redirections silently as before.

    aaron> As I understood it, no redirections are ever being
    aaron> handled silently anymore.

I said *previous*. The *previous* implementation handled
redirections silently *except* for BzrDirFormat1.

    aaron> Further, I think that if any control directory's
    aaron> find_format method raises a RedirectRested,
    aaron> redirecting is the only reasonable choice.  A control
    aaron> dir that does not want this processing should handle
    aaron> the redirect in find_format.
    >> 
    >> You cannot redefine find_format in a daughter class. Well, you
    >> can, it will just be never called (that's precisely the point a
    >> registering the concrete class).

    aaron> Okay, so it's probe_transport where they would need to
    aaron> handle a redirect differently.

Except that probe_transport returns a format and do not provide a
mean to change the transport, hence open_from_transport should
catch the exception and change the transport.

    >> I reverted it then but the point remains: if we are not able to
    >> define specific probe_transport for bzr, one day or another some
    >> refactoring will be in order.

    aaron> The specific probe_transport for bzr is
    aaron> BzrDirFormat.probe_transport.

Ok ;-)

    aaron> I don't see any test to ensure that redirecting a
    aaron> bundle works.  We should have one of those.  It might
    aaron> also be nice to test do_catching_redirections.

Good point, I thought the other tests cover enough of it, I was
wrong.

More tests included in this patch ;-)

<snip/>

  >> +    else:
    >> +        # Loop exited without resolving redirect ? Either the user has kept
    >> +        # a very very very old reference or a loop occured in the
    >> +        # redirections.  Nothing we can cure here: tell the user. Note that
    >> +        # as the user has been informed about each redirection (it is the
    >> +        # caller responsibility to do that in redirected via the provided
    >> +        # redirection_notice), there is no need to issue an additional error
    >> +        # message.
    >> +        raise exception

    aaron> I think it would make sense to raise TooManyRedirections here, and let
    aaron> callers re-raise as something else, if appropriate.

Hmmm, yeah, nicer.

    >> @@ -226,12 +226,13 @@
    >> code, response_file = self._get(relpath, None)
    >> return response_file
    >> 
    >> -    def _get(self, relpath, ranges):
    >> +    def _get(self, relpath, ranges, tail_amount=0):
    >> """Get a file, or part of a file.
    >> 
    >> :param relpath: Path relative to transport base URL
    >> -        :param byte_range: None to get the whole file;
    >> -            or [(start,end)] to fetch parts of a file.
    >> +        :param ranges: None to get the whole file;
    >> +            or [(start,end)+], a list of tuples to fetch parts of a file.
    >> +        :param tail_amount: to fetch that amount from file tail.

    aaron> This documentation doesn't clarify what tail_amount is, to me.

Hope my new try is better.

I also merge the fix for bug 88780 but time is running short :(
I'll be on vacations for a week, so I let you decide if this
patch is worth merging now or should wait for the next cycle.

In any case, I think the 88780 fix should be merged and I promise
I'll write the tests anyway ! :-)

     Vincent

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 185 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070301/d3dd2ebb/attachment-0001.pgp 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr.http.redirection.patch
Type: text/x-patch
Size: 490347 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070301/d3dd2ebb/attachment-0001.bin