launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> === modified file 'lib/lp/registry/interfaces/ssh.py'
> --- lib/lp/registry/interfaces/ssh.py	2018-02-24 09:11:39 +0000
> +++ lib/lp/registry/interfaces/ssh.py	2018-06-19 21:32:30 +0000
> @@ -124,4 +124,21 @@
>  
>  @error_status(httplib.BAD_REQUEST)
>  class SSHKeyAdditionError(Exception):
> -    """Raised when the SSH public key is invalid."""
> +    """Raised when the SSH public key is invalid.
> +
> +    WARNING: Be careful when changing the message format as
> +    SSO uses a regex to recognize it.
> +    """
> +
> +    def __init__(self, *args, **kwargs):
> +        msg = ""
> +        if 'key' in kwargs:
> +            key = kwargs.pop('key')
> +            msg = "Invalid SSH key data: '%s'" % key
> +        if 'kind' in kwargs:
> +            kind = kwargs.pop('kind')
> +            msg = "Invalid SSH key type: '%s'" % kind
> +        if 'exception' in kwargs:
> +            exception = kwargs.pop('exception')
> +            msg = "%s (%s)" % (msg, str(exception).decode('utf-8', 'ignore'))

This is going to be somewhat unhelpful when debugging the resulting error, and it will also be an obstacle to Python 3 porting.  How about this instead?

  try:
      exception_text = six.text_type(exception)
  except UnicodeDecodeError:
      # On Python 2, Key.fromString can raise exceptions with
      # non-UTF-8 messages.
      exception_text = bytes(exception).decode('unicode_escape')
  msg = "%s (%s)" % (msg, exception_text)

In your test case below, I think that produces "Invalid SSH key data: 'ssh-rsa asdfasdf comment' (unknown blob type: \xc7_)" on Python 2 and "Invalid SSH key data: 'ssh-rsa asdfasdf comment' (unknown blob type: b'\xc7_')" on Python 3 (not a perfect match, but close enough!), which seems a lot more helpful.

> +        super(SSHKeyAdditionError, self).__init__(msg, *args, **kwargs)


-- 
https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad.