[MERGE][1.6][#254797] Fix NotImplementedError when probing for smart protocol via HTTP.

John Arbash Meinel john at arbash-meinel.com
Thu Aug 14 04:22:03 BST 2008


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

Andrew Bennetts wrote:
> This fixes a regression introduced in 1.6.  Probing for a smart server over HTTP was
> triggering a NotImplementedError, because read_line was not implemented on the
> HTTP medium request class.
> 
> This fix implements that method, and does some very simple refactoring to do so
> without duplicating code.
> 
> It also adds a simple test to test_http that fails without this fix.
> Implementing that test required improving the HTTPServerWithSmarts test helper
> to behave a little more realistically.
> 
> Despite the size of the diff, this is a simple and I believe safe change to
> incorporate in 1.6 final.  The bulk of the diff affects code that was not
> working without this fix, or tests for that fix.  The rest is a straightforward
> refactoring of code that has extensive test coverage.
> 
> -Andrew.
> 
> 

The only thing I don't really like is:

+    def _read_line(self):
+        line, excess = medium._get_line(self._response_body.read)
+        if excess != '':
+            raise AssertionError(
+                '_get_line returned excess bytes, but this mediumrequest '
+                'cannot handle excess. (%r)' % (excess,))
+        return line
+

^- you are using AssertionError to validate data from the wire, and the code
path is completely untested.

I'll let it pass, but I personally like to see failure code *exercised* by the
test suite.

The test also seems a bit indirect. I realize it probably would fail to
BzrDir.open('smart+http://') but it doesn't really feel like you are testing
that Medium's are implementing the appropriate interface.

Anyway, this seems like a minimal fix for 1.6, and there isn't anything I
would strictly block on. And it mostly comes back to wanting
"smart_medium_implementations/" testing.

So...

BB:approve

And I'll get it queued up for 1.6.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIo6TbJdeBCYSNAAMRAqYGAKDZ+AJbM6xK14afVY+OHGk/vHrtXQCgsjIf
zFrsZ7EMPE2Vlpki0dcppsQ=
=eF+2
-----END PGP SIGNATURE-----



More information about the bazaar mailing list