Code review comment for lp:~leonardr/launchpadlib/anonymous-access

Revision history for this message
Guilherme Salgado (salgado) wrote :

Hi Leonard,

Just a couple nitpicks, but it looks good to go.

 status approved
 review approve

On Tue, 2009-12-15 at 18:33 +0000, Leonard Richardson wrote:
> Leonard Richardson has proposed merging

> === modified file 'src/launchpadlib/credentials.py'
> --- src/launchpadlib/credentials.py 2009-11-02 21:03:53 +0000
> +++ src/launchpadlib/credentials.py 2009-12-15 18:33:17 +0000
> @@ -19,6 +19,7 @@
> __metaclass__ = type
> __all__ = [
> 'AccessToken',
> + 'AnonymousAccessToken',
> 'RequestTokenAuthorizationEngine',
> 'Consumer',
> 'Credentials',
> @@ -169,6 +170,14 @@
> return cls(key, secret, context)
>
>
> +class AnonymousAccessToken(_AccessToken):
> + """An OAuth access token that doesn't authenticate anybody.
> +
> + This token can be used for anonymous access."""

PEP-8 says the closing triple quotes should be on a line by itself for
multi-line docstrings.

> + def __init__(self):
> + super(AnonymousAccessToken, self).__init__('','')
> +
> +
> class SimulatedLaunchpadBrowser(object):
> """A programmable substitute for a human-operated web browser.
>

> === modified file 'src/launchpadlib/launchpad.py'
> --- src/launchpadlib/launchpad.py 2009-12-03 14:38:29 +0000
> +++ src/launchpadlib/launchpad.py 2009-12-15 18:33:17 +0000
> @@ -32,7 +32,8 @@
> from lazr.restfulclient.resource import (
> CollectionWithKeyBasedLookup, HostedFile, ServiceRoot)
> from launchpadlib.credentials import (
> - AccessToken, Credentials, AuthorizeRequestTokenWithBrowser)
> + AccessToken, AnonymousAccessToken, Credentials,
> + AuthorizeRequestTokenWithBrowser)
> from oauth.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT
> from launchpadlib import uris
>
> @@ -170,6 +171,19 @@
> return cls(credentials, service_root, cache, timeout, proxy_info)
>
> @classmethod
> + def login_anonymously(
> + cls, consumer_name, service_root=uris.STAGING_SERVICE_ROOT,
> + launchpadlib_dir=None, timeout=None, proxy_info=None):
> + """Get access to Launchpad without providing any credentials.
> +
> + """

You could fit that whole docstring on a single line, no?

> + service_root_dir, cache_path = cls._get_paths(
> + service_root, launchpadlib_dir)
> + token = AnonymousAccessToken()
> + credentials = Credentials(consumer_name, access_token=token)
> + return cls(credentials, service_root, cache_path, timeout, proxy_info)
> +
> + @classmethod
> def login_with(cls, consumer_name,
> service_root=uris.STAGING_SERVICE_ROOT,
> launchpadlib_dir=None, timeout=None, proxy_info=None,

--
Guilherme Salgado <email address hidden>

review: Approve

« Back to merge proposal