Merge lp:~pfalcon/linaro-android-mirror/xmlrpc-error-msg into lp:linaro-android-mirror

[Paul Sokolovsky]

Hello Michael,

Here's patch to actually report any exception happened during mirroring down to client, because so far client receive generic error, e.g. https://android-build.linaro.org/jenkins/job/linaro-android_leb-panda/36/console

It also shows kind of issues I have with Twisted - it has strange defaults (not passing error content down to client), and to work that around, one have to dig in internal classes. Or maybe chain error callback thru the web of code, but that's exactly why exceptions were invented - to avoid that. Anyway, I based this solution on discussion at and around http://twistedmatrix.com/pipermail/twisted-python/2005-April/010248.html . Please see if this is good enough or is there other way to do it.

Thanks,
Paul

[Michael Hudson-Doyle (mwhudson)]

I think the approach in lp:~mwhudson/linaro-android-mirror/xmlrpc-error-msg is a bit cleaner. I agree that DeferredList and XMLRPC error reporting are both pretty awkward though.

[Paul Sokolovsky (pfalcon)]

> I think the approach in lp:~mwhudson/linaro-android-mirror/xmlrpc-error-msg is
> a bit cleaner. I agree that DeferredList and XMLRPC error reporting are both
> pretty awkward though.

So, should that implementation be used instead of mine, because it seems to be applied on top? Also, from my, non-twisted-expert view, it has the same traits (uses a private method, fishes for stuff like failure.value.subFailure), but also adds more: deeper black twisted magic, overloads semantics of standard method. All in all, Michael, if that's the right way to do it, can you please merge it the way it should be (instead or on top of my changes). Thanks!

[Paul Sokolovsky (pfalcon)]

Ok, we're having issues due to this in realtime (developers can't get build up due to mirror errors, and cannot see the actual errors, and need to recurse to me for looking them up). So, I'll merge this patch as knowing working, let's elaborate it as needed afterwards.

=== modified file 'linaro_android_mirror/service.py'
--- linaro_android_mirror/service.py	2011-04-18 13:02:12 +0000
+++ linaro_android_mirror/service.py	2011-05-20 10:57:35 +0000
@@ -1,14 +1,30 @@
 import time
+from xmlrpclib import Fault
 
 from twisted.web.xmlrpc import XMLRPC
 from twisted.python import log
+from twisted.internet.defer import FirstError
 
 from linaro_android_mirror.mirrorer import mirror
 from linaro_android_mirror.factor_manifest import make_slave_manifest
 from . import settings
 
 
-class MirrorEndpoint(XMLRPC):
+class ExceptionPropogatingXMLRPC(XMLRPC):
+    "XMLRPC subclass which returns error message to the client."
+
+    # See http://twistedmatrix.com/pipermail/twisted-python/2005-April/010248.html
+    def _ebRender(self, failure):
+        if isinstance(failure.value, Fault):
+            return failure.value
+        # Unwrap original exception which was thrown
+        exc = failure.value
+        if isinstance(exc, FirstError):
+            exc = exc.subFailure.value
+        return Fault(self.FAILURE, str(exc))
+
+
+class MirrorEndpoint(ExceptionPropogatingXMLRPC):
 
     def xmlrpc_mirror(self, text):
         log.msg("mirror request: in %d bytes" % len(text))
 
=== added file 'linaro_android_mirror/tests/data/manifest-bad-loc.xml'
--- linaro_android_mirror/tests/data/manifest-bad-loc.xml	1970-01-01 00:00:00 +0000
+++ linaro_android_mirror/tests/data/manifest-bad-loc.xml	2011-05-20 10:57:35 +0000
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<manifest>
+  <remote fetch="git://git.linaro.org/" name="linaro"/>
+  <project name="theres/no/such/location" path="hardware/libhardware" remote="linaro" revision="master" />
+</manifest>
 
=== added file 'linaro_android_mirror/tests/data/manifest-no-rev.xml'
--- linaro_android_mirror/tests/data/manifest-no-rev.xml	1970-01-01 00:00:00 +0000
+++ linaro_android_mirror/tests/data/manifest-no-rev.xml	2011-05-20 10:57:35 +0000
@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<manifest>
+  <remote fetch="git://git.linaro.org/" name="linaro"/>
+  <project name="theres/no/such/location" path="hardware/libhardware" remote="linaro"/>
+</manifest>
 
=== added file 'linaro_android_mirror/tests/test_mirrorer_integrational.py'
--- linaro_android_mirror/tests/test_mirrorer_integrational.py	1970-01-01 00:00:00 +0000
+++ linaro_android_mirror/tests/test_mirrorer_integrational.py	2011-05-20 10:57:35 +0000
@@ -0,0 +1,21 @@
+import os
+import sys
+import xmlrpclib
+
+# Integrational test for mirror service, expects server to be running at localhost
+
+def _test_for_fault(manifest_file, error_substr):
+    service = xmlrpclib.ServerProxy("http://localhost:8080")
+    basedir = os.path.dirname(__file__)
+    manifest = open(os.path.join(basedir, manifest_file)).read()
+    try:
+        service.mirror(manifest)
+        assert False, "Expected exception was not raised"
+    except xmlrpclib.Fault, e:
+        assert error_substr in e.faultString
+
+def test_bad_manifest_git_loc():
+    _test_for_fault("data/manifest-bad-loc.xml", "Cannot fetch theres/no/such/location")
+
+def test_bad_manifest():
+    _test_for_fault("data/manifest-no-rev.xml", "no revision for project theres/no/such/location")