[MERGE] HTTP redirection

[Vincent Ladeuil]

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

    aaron> Vincent Ladeuil wrote:
    >> After discussions with Robert at NlSprint, John on IRC and Robert
    >> again on IRC, here is a new attempt at http redirections.

    aaron> I think this is very close to mergeable.

    >> - By default http do not redirect silently but instead raise the
    >> RedirectedRequest exception,

    aaron> I'm a little sad to see that removed, because I use
    aaron> transports elsewhere (e.g. 'bzr patch' from bzrtools)
    aaron> and I'll have to manually handle redirects now.

See my other mail, I think you have nothing to do.

Except for the site redirection case, I'm not sure there exist
*bzr* uses for the http redirections, a BzrDir cannot be split
across several sites, and neither can a shared repository and its
associated branches.

Now if you use an http redirection to provide the right file name
for the patch in Bundle Buggy, then, yes, we should help you help
us ;-)

<snip/>

    aaron> I think this would be more clearly handled as a for..else loop:

Indeed ! I found my loop awkward but didn't know about the else
clause, perfect match... (and thanks too for having caught the
grammar errors in the comments).

    >> @@ -1086,7 +1128,8 @@
    >> def probe_transport(klass, transport):
    >> """Return the .bzrdir style transport present at URL."""
    >> try:
    >> -            format_string = transport.get(".bzr/branch-format").read()
    >> +            format_file = transport.get(".bzr/branch-format")
    >> +            format_string = format_file.read()

    aaron> ^^^ I don't understand this change.

Sorry, noise, intermediate debug state :-)

    >> -# register BzrDirFormat as a control format
    >> -BzrDirFormat.register_control_format(BzrDirFormat)
    >> -
    >> -
    >> class BzrDirFormat4(BzrDirFormat):
    >> """Bzr dir format 4.
    >> 
    >> @@ -1475,6 +1514,11 @@
    >> repository_format = property(__return_repository_format, __set_repository_format)
    >> 
    >> 
    >> +# register BzrDirMetaFormat1 as a control format. Our only
    >> +# concrete control format (BzrDirFormat is an abstract one).
    >> +BzrDirFormat.register_control_format(BzrDirMetaFormat1)
    >> +
    >> +# Register bzr formats
    >> BzrDirFormat.register_format(BzrDirFormat4())
    >> BzrDirFormat.register_format(BzrDirFormat5())
    >> BzrDirFormat.register_format(BzrDirFormat6())

    aaron> Comment ("our only concrete control format") seems to be contradicted by
    aaron> registering BzrDirFormat4, 5, 6.


*control* format not BzrDir format, or am I misunderstanding
something (the truth is the distinction is a bit blurry for me,
but registries are distinct...) ?


    aaron> I think we should be testing that redirecting bzrdirs
    aaron> works with BzrDir.open, 
    aaron> i.e. that the resulting transport base takes the
    aaron> redirects into account.  It probably makes sense to
    aaron> test open, open_from_transport,
    aaron> open_containing_from_transport, and open_containing.

open, open_containing and open_containing_from_transport all end
up calling open_from_transport and use the resulting BzrDir
(which has two attributes transport and root_transport, both
constructed from the transport probed by the format, i.e. the
final target of the redirections).

What will be the aim of the additional tests ? Again, I may be
misunderstanding you and the test policy.

The tests I have written ensure that the http transport handle
the redirections and that BzrDir detect a loop or a too deep
redirection chain. I thought that covered my modifications.

    aaron> We should make a clear statement about how
    aaron> open_containing works: does it follow the initial
    aaron> redirect, or does it keep on altering the initial url
    aaron> until BzrDir.open succeeds (by following a redirect to
    aaron> a real branch)?

open_containing_from_transport have not changed. It tries all
directories from the initial url to the root until it finds a
branch or fail.

What have changed is that for each directory:

- before: it silently followed the redirections until it reached
  the target (at the transport level), and checked the format.
 
- now: it follows the redirections until it reaches the target
  and check the format.

As before I just attached the diff, the merge
can be done from the
http://bazaar.launchpad.net/~bzr/bzr/bzr.http.redirection branch.

Have I answered your concerns ?

  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/20070213/dde84119/attachment-0001.pgp 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr.http.redirection.patch
Type: text/x-patch
Size: 30466 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070213/dde84119/attachment-0001.bin