[merge] Enable writable http transports

Vincent Ladeuil v.ladeuil at alplog.fr
Wed Sep 13 08:47:10 BST 2006


>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:

<snip/>

    >> === modified file bzrlib/transport/__init__.py
    >> --- bzrlib/transport/__init__.py
    >> +++ bzrlib/transport/__init__.py
    >> @@ -586,6 +586,8 @@
    >> create it, and then try again.
    >> :param dir_mode: Possible access permissions for new directories.
    >> """
    >> +        # Keep file position in case something goes wrong at first put_file try
    >> +        f_pos = f.tell()
    >> # Default implementation just does an atomic put.
    >> try:
    >> return self.put_file(relpath, f, mode=mode)
    >> @@ -595,6 +597,7 @@
    >> parent_dir = osutils.dirname(relpath)
    >> if parent_dir:
    >> self.mkdir(parent_dir, mode=dir_mode)
    >> +                f.seek(f_pos)
    >> return self.put_file(relpath, f, mode=mode)

    jam> ^- Vila and I discussed this a little bit on IRC.  The
    jam> problem is that urllib directly returns a socket, which
    jam> doesn't have tell() or seek(). So far, we haven't had
    jam> any problems without it, because SFTP and Local both
    jam> fail at open() time, not on the first write.  However
    jam> HTTP actually fails after trying to send the contents of
    jam> the files.

    jam> The webdav plugin itself works around this by using a
    jam> different implementation of put_file_non_atomic() (It
    jam> implements _bytes, and just reads the file 1 time).

    jam> I would be interested to hear from the group,
    jam> though. Whether we would prefer to require transports to
    jam> fail on put() without changing the file pointer, or for
    jam> the file-like objects to have tell()/seek() members.  Or
    jam> just require that any transport that would modify the
    jam> file pointer implement custom handlers for stuff like
    jam> this.

Yeah, as in create a local temporary copy of the files (I know,
can of worms, etc, just trying to expose possible solutions).

<snip/>

    >> def __init__(self, base):
    >> """Set the base path where files will be stored."""
    >> -        proto_match = re.match(r'^(https?)(\+\w+)?://', base)
    >> +        proto_match = re.match(r'^(https?)(\+\w+)*://', base)

    jam> ^- This is so you can have multiple levels, right? "http+foo+bar+baz://"

Exactly, during a transition period, I had http+webdav+pycurl and
http+webdav+urllib. Since then I discover another occurrence of
the problem in the extract_auth (I think), but as I have switched
to urllib only for webdav that's not a concern anymore.

    >> if not proto_match:
    >> raise AssertionError("not a http url: %r" % base)
    >> self._proto = proto_match.group(1)
    >> @@ -321,7 +321,7 @@
    >> # At this point HttpTransport might be able to check and see if
    >> # the remote location is the same, and rather than download, and
    >> # then upload, it could just issue a remote copy_this command.
    >> -        if isinstance(other, HttpTransportBase):
    >> +        if isinstance(other, HttpTransportBase) and other.is_readonly():
    >> raise TransportNotPossible('http cannot be the target of copy_to()')
    >> else:
    >> return super(HttpTransportBase, self).\


    jam> ^- We also talked about this on IRC, and we decided it
    jam> was better for Webdav to just re-implement
    jam> 'copy_to()'. However, I realize now that it isn't the
    jam> case.

    jam> Because you could have a plain HTTP transport trying to
    jam> 'copy_to()' a WebDav transport.  I think a better
    jam> solution would be to just remove 'copy_to' from
    jam> HttpTransportBase completely, and then you'll just get
    jam> the standard error from 'put()' about not being able to
    jam> write to the remote transport. No reason to short-cut
    jam> early when the results may be wrong :)

Ok.

        Vincent




More information about the bazaar mailing list