[MERGE] Enable writable http transports

Aaron Bentley aaron.bentley at utoronto.ca
Wed Aug 30 15:06:26 BST 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent LADEUIL wrote:
> (TransportTests.test_rmdir): Use 'has' to test the existence of
> the file instead of stat.  A transport may be writable without
> implementing stat.  The intent of the test is to test rmdir not
> stat.

Transport.has itself can be quite flaky on http.  Perhaps calling rmdir
a second time, and asserting that it raises NoSuchFile?

> @@ -543,9 +543,9 @@
>          t.mkdir('adir')
>          t.mkdir('adir/bdir')
>          t.rmdir('adir/bdir')
> -        self.assertRaises(NoSuchFile, t.stat, 'adir/bdir')
> +        self.failIf(t.has('adir/bdir'))

We use the assert family, so assertFalse would be preferable here.

This bzrlib/transport/__init__.py change looks spurious (and not PEP8):

> === modified file bzrlib/transport/__init__.py // last-changed:v.ladeuil at alplog
> ... .fr-20060828163142-b9e01629153529b9
> --- bzrlib/transport/__init__.py
> +++ bzrlib/transport/__init__.py
> @@ -253,6 +253,7 @@
>          else:
>              pumpfile(from_file, to_file)
>  
> +
>      def _get_total(self, multi):
>          """Try to figure out how many entries are in multi,
>          but if not possible, return None.
> 


> +	    # read only transports never manipulate directories
> +            if self.is_readonly() and relpath_parts[-1] == '':

Does your plugin open all http branches, or does it use the plain http
handlers for non WebDAV urls?  Probably doesn't matter much.  The point
is to test that for readonly operations, directories aren't used, so as
long as the readonly transport is also under test, we should be okay.

Could you please not name your branch bzr.webdav/trunk?  The last
element of the name should be unique to your project, because that's the
default name used in various places.  Ideally, the last element would be
the plugin name, i.e. "bzr/webdav" as in "bzrlib.plugins.webdav".
"trunk/bzr.webdav" or "bzr.webdav.trunk" would also be fine.

>                  raise ValueError("path %r within branch %r seems to be a directory"
>                                   % (relpath, self._path))
>          basepath = self._path.split('/')
> @@ -494,10 +497,14 @@
>      # used to form the url that connects to this server
>      _url_protocol = 'http'
>  
> +    # Subclasses can provide a specific request handler
> +    def __init__(self, request_handler=TestingHTTPRequestHandler):
> +        self.request_handler = request_handler

It's probably a good idea to call Server.__init__ in here (it may do
something, someday).

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFE9Zti0F+nu1YWqI0RAm8RAJ0crAsL7JtREyK7ZXu44AsazYdzmACfdvsv
wWTldmtRqPaogR33ozp8kjA=
=Gnrw
-----END PGP SIGNATURE-----




More information about the bazaar mailing list