Patchwork xmlrpc: Avoid trying to marshall None for people with no name

login
register
mail settings
Submitter Doug Anderson
Date Dec. 21, 2012, 11:42 p.m.
Message ID <1356133363-7409-1-git-send-email-dianders@chromium.org>
Download mbox | patch
Permalink /patch/207898/
State Accepted
Headers show

Comments

Doug Anderson - Dec. 21, 2012, 11:42 p.m.
If a person sent patches with no real name, they may have None for
their name.  In this case just use their email address as their name.

The previous pwclient error that would show up looks like:

  $ pwclient list -w anonymous@example.com
  Traceback (most recent call last):
    File ".../pwclient", line 631, in <module>
      main()
    File ".../pwclient", line 570, in main
      action_list(rpc, filt, submitter_str, delegate_str, series_str)
    File ".../pwclient", line 316, in action_list
      ids = person_ids_by_name(rpc, submitter_str)
    File ".../pwclient", line 241, in person_ids_by_name
      people = rpc.person_list(name, 0)
    File "/usr/lib/python2.7/xmlrpclib.py", line 1224, in __call__
      return self.__send(self.__name, args)
    File "/usr/lib/python2.7/xmlrpclib.py", line 1578, in __request
      verbose=self.__verbose
    File "/usr/lib/python2.7/xmlrpclib.py", line 1264, in request
      return self.single_request(host, handler, request_body, verbose)
    File "/usr/lib/python2.7/xmlrpclib.py", line 1297, in single_request
      return self.parse_response(response)
    File "/usr/lib/python2.7/xmlrpclib.py", line 1473, in parse_response
      return u.close()
    File "/usr/lib/python2.7/xmlrpclib.py", line 793, in close
      raise Fault(**self._stack[0])
  xmlrpclib.Fault: <Fault 1: "<type 'exceptions.TypeError'>:cannot marshal None unless allow_none is enabled">

This is the server side fix to the pwclient workaround I send out
titled "pwclient: Handle servers that barf on users with no name"

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 apps/patchwork/views/xmlrpc.py |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Jeremy Kerr - Dec. 30, 2012, 1:41 a.m.
Hi Doug,

> This is the server side fix to the pwclient workaround I send out
> titled "pwclient: Handle servers that barf on users with no name"

So we won't need the pwclient workaround with this fix applied, right?

Cheers,


Jeremy
Doug Anderson - Dec. 30, 2012, 4:41 a.m.
Jeremy,


On Sat, Dec 29, 2012 at 7:41 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Doug,
>
>
>> This is the server side fix to the pwclient workaround I send out
>> titled "pwclient: Handle servers that barf on users with no name"
>
>
> So we won't need the pwclient workaround with this fix applied, right?

Correct.  ...well, at least once this fix rolls out to all relevant
servers.  Until that happen I'll probably keep the client patch around
in my local checkout.  ...but the client fix is pretty hacky so if we
think that important servers will be upgraded before too long it's
probably best to abandon the client fix.

-Doug
Jeremy Kerr - Dec. 30, 2012, 4:47 a.m.
Hi Doug,

> Correct.  ...well, at least once this fix rolls out to all relevant
> servers.  Until that happen I'll probably keep the client patch around
> in my local checkout.  ...but the client fix is pretty hacky so if we
> think that important servers will be upgraded before too long it's
> probably best to abandon the client fix.

OK, sounds good. Are you still using the patch:

  pwclient: Allow using a submitter id to filter

as a workaround only, or is it something you'd like applied anyway? The 
submitter IDs aren't generally visible.

Cheers,


Jeremy
Doug Anderson - Dec. 30, 2012, 4:58 a.m.
On Sat, Dec 29, 2012 at 8:47 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
> Hi Doug,
>
>
>> Correct.  ...well, at least once this fix rolls out to all relevant
>> servers.  Until that happen I'll probably keep the client patch around
>> in my local checkout.  ...but the client fix is pretty hacky so if we
>> think that important servers will be upgraded before too long it's
>> probably best to abandon the client fix.
>
>
> OK, sounds good. Are you still using the patch:
>
>  pwclient: Allow using a submitter id to filter
>
> as a workaround only, or is it something you'd like applied anyway? The
> submitter IDs aren't generally visible.

Well, the "info" command now shows them (which is OK I think), so it's
somewhat visible now.  ...but yeah, the original motivation for
allowing the submitter ID was to workaround the XMLRPC bug.

Dropping it is fine with me--no need to add extra cruft when it's not needed.

-Doug

Patch

diff --git a/apps/patchwork/views/xmlrpc.py b/apps/patchwork/views/xmlrpc.py
index 283eb34..a69c858 100644
--- a/apps/patchwork/views/xmlrpc.py
+++ b/apps/patchwork/views/xmlrpc.py
@@ -167,11 +167,19 @@  def project_to_dict(obj):
 def person_to_dict(obj):
     """Return a trimmed down dictionary representation of a Person
     object which is OK to send to the client."""
+
+    # Make sure we don't return None even if the user submitted a patch
+    # with no real name.  XMLRPC can't marshall None.
+    if obj.name is not None:
+        name = obj.name
+    else:
+        name = obj.email
+
     return \
         {
          'id'           : obj.id,
          'email'        : obj.email,
-         'name'         : obj.name,
+         'name'         : name,
          'user'         : unicode(obj.user).encode("utf-8"),
         }