Imminent MERGE request: 12 steps towards a high performance server

[John Arbash Meinel]

...

>    http://people.ubuntu.com/~andrew/bzr/extra-transport-tests/
>      this adds a bunch of tests for transports, particularly about how the path
>      handling at the root of a url works -- and fixes the bugs they reveal.

-            self.assertEquals(str(e),
-                    'Transport error: ~janneke: invalid port number ')
+            self.assertEquals(str(e),
+                    'Transport error: '
+                    'invalid port number ~janneke in url '
+                    'sftp://lilypond.org:~janneke/public_html/bzr/gub ')
+##             self.assertEquals(str(e),
+##                     'Transport error: ~janneke: invalid port number ')

Don't comment out the old lines, just delete them. (I understand
commenting while you are getting it written, but in the long term, just
get rid of it)

Also, URLs are usually long things, such that if they are going to be
user-facing, I would rather see them on a line of their own. Something like:

Transport error: invalid port number ~janneke in url:
sftp://lilypond.org:~janneke/public_html/bzr/gub

Also, why is there a blank space at the end of the error string? you have:

'.../bzr/gub '
            ^- this is the space in question.


I'm not sure that this test is valid:
+        self.assertRaises(NoSuchFile,
+                          t.put, 'path/doesnt/exist/c',
StringIO('contents'))
+

At the very least, since put() is now deprecated, this will be spewing a
deprecated() parameter to the screen. I don't think it is really needed
anymore, but you can tell me if you feel differently.

You seem to be adding a 'put_permissions' test, even though 'put()' is a
deprecated API. I think this is unnecessary. If you want, you can do a
single test, to make sure that put() is taking a mode, and passing it to
put_file() or put_bytes(). But I don't think we need to test the full
set of modes.


The http/__init__.py changes:

I'm not sure that it should be disallowed for the path passed to
'abspath()' to be a directory. The associated message seems to be:

	- http url fixes suggested by Robey Pointer, and tests

And committed by Martin.

Anyway, your actual changes seem okay. If you can justify why you needed
the extra put() tests. The http stuff is just something that we should
figure out why we want to disallow a trailing slash passed to abspath. I
realize it isn't the standard way of using it anyway (even if the
parameter is a directory, you would pass it without the trailing slash).

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060913/64c531fd/attachment.pgp