launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/launchpad_loggerhead/app.py'
> --- lib/launchpad_loggerhead/app.py	2014-02-19 02:38:39 +0000
> +++ lib/launchpad_loggerhead/app.py	2015-05-12 14:51:43 +0000
> @@ -150,6 +150,7 @@
>                    "logged in as the right user, or log into Launchpad and try "
>                    "again.")
>                  raise exc
> +            environ[self.session_var]['identity_url'] = response.identity_url
>              environ[self.session_var]['user'] = sreg_info['nickname']

This can probably be dropped now.

>              raise HTTPMovedPermanently(query['back_to'])
>          elif response.status == FAILURE:
> @@ -195,15 +196,18 @@
>              return self._logout(environ, start_response)
>          path = environ['PATH_INFO']
>          trailingSlashCount = len(path) - len(path.rstrip('/'))
> +        identity_url = environ[self.session_var].get(
> +            'identity_url', LAUNCHPAD_ANONYMOUS)
>          user = environ[self.session_var].get('user', LAUNCHPAD_ANONYMOUS)
> -        lp_server = get_lp_server(user, branch_transport=self.get_transport())
> +        lp_server = get_lp_server(
> +            identity_url, branch_transport=self.get_transport())
>          lp_server.start_server()
>          try:
>  
>              try:
>                  branchfs = self.get_branchfs()
>                  transport_type, info, trail = branchfs.translatePath(
> -                    user, urlutils.escape(path))
> +                    identity_url, urlutils.escape(path))
>              except xmlrpclib.Fault as f:
>                  if check_fault(f, faults.PathTranslationError):
>                      raise HTTPNotFound()
> 
> === modified file 'lib/lp/code/xmlrpc/codehosting.py'
> --- lib/lp/code/xmlrpc/codehosting.py	2015-04-13 19:12:23 +0000
> +++ lib/lp/code/xmlrpc/codehosting.py	2015-05-12 14:51:43 +0000
> @@ -112,6 +112,8 @@
>      if isinstance(login_id, basestring):
>          # OpenID identifiers must contain a slash, while names must not.
>          if "/" in login_id:
> +            if isinstance(login_id, bytes):
> +                login_id = login_id.decode("UTF-8")

Should this apply to the username case too?

>              requester = getUtility(IPersonSet).getByOpenIDIdentifier(login_id)
>          else:
>              requester = getUtility(IPersonSet).getByName(login_id)
> 
> === modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
> --- lib/lp/code/xmlrpc/tests/test_codehosting.py	2015-04-08 12:36:04 +0000
> +++ lib/lp/code/xmlrpc/tests/test_codehosting.py	2015-05-12 14:51:43 +0000
> @@ -110,7 +110,8 @@
>              identifier = (
>                  self.person.account.openid_identifiers.one().identifier)
>          username = run_with_login(
> -            u'http://testopenid.dev/+id/%s' % identifier,
> +            # Deliberately not Unicode, since XML-RPC gives us a byte string.
> +            (u'http://testopenid.dev/+id/%s' % identifier).encode("UTF-8"),
>              get_logged_in_username)
>          login(ANONYMOUS)
>          self.assertEqual(self.person.name, username)
> 


-- 
https://code.launchpad.net/~cjwatson/launchpad/loggerhead-openid/+merge/258891
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.