[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