launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> === modified file 'lib/lp/soyuz/interfaces/archive.py'
> --- lib/lp/soyuz/interfaces/archive.py	2016-01-26 15:47:37 +0000
> +++ lib/lp/soyuz/interfaces/archive.py	2016-07-07 14:30:49 +0000
> @@ -2074,6 +2087,57 @@
>          :param dependency: is an `IArchive` object.
>          """
>  
> +    @operation_parameters(
> +        name=TextLine(title=_("Authorization token name"), required=True))

Keep token:

<wgrant> My only other question is whether we should allow the secret to be fixed.
<cjwatson> You mean allowing the caller to specify the token?
<cjwatson> I don't see the gain in that.
<wgrant> It was used once upon a time for a hack (the font hack? or U1, or something)
<wgrant> And it allows mistakes to potentially be fixed.
<cjwatson> I guess it's fine as long as it's documented as "normally leave this unset".
<wgrant> Yep
<cjwatson> I've revised the spec.

I don't mind whether you keep date_created or not; that sort of thing is typically used in tests, but if you have no tests that use it then dropping it is fine.

> +    @export_write_operation()
> +    @operation_for_version("devel")
> +    def newNamedAuthToken(name, token=None, date_created=None):
> +        """Create a new named authorization token.
> +
> +        :param name: An identifier string for this token.
> +        :param token: Optional unicode text to use as the token. One will be
> +            generated if not given.
> +        :param date_created: Optional, defaults to now.
> +
> +        :return: A dictionary where the value of `token` is the secret and
> +            the value of `archive_url` is the externally-usable archive URL
> +            including basic auth.
> +        """
> +
> +    @operation_parameters(
> +        name=TextLine(title=_("Authorization token name"), required=True))
> +    @export_read_operation()
> +    @operation_for_version("devel")
> +    def getNamedAuthToken(name):
> +        """Return a named authorization token for a given archive and name.
> +
> +        :param name: The identifier string for a token.
> +
> +        :return: A dictionary where the value of `token` is the secret and
> +            the value of `archive_url` is the externally-usable archive URL
> +            including basic auth.
> +        """
> +
> +    @export_read_operation()
> +    @operation_for_version("devel")
> +    def getNamedAuthTokens():
> +        """Return a list of named authorization tokens for a given archive.
> +
> +        :return: A list of dictionaries where the value of `token` is the
> +            secret and the value of `archive_url` is the externally-usable
> +            archive URL including basic auth.
> +        """
> +
> +    @operation_parameters(
> +        name=TextLine(title=_("Authorization token name"), required=True))
> +    @export_write_operation()
> +    @operation_for_version("devel")
> +    def revokeNamedAuthToken(name):
> +        """Deactivates a named authorization token.
> +
> +        :param name: The identifier string for a token.
> +        """
> +
>  
>  class IArchiveAdmin(Interface):
>      """Archive interface for operations restricted by commercial."""
> 
> === modified file 'lib/lp/testing/__init__.py'
> --- lib/lp/testing/__init__.py	2015-11-08 01:05:24 +0000
> +++ lib/lp/testing/__init__.py	2016-07-07 14:30:49 +0000
> @@ -619,7 +619,7 @@
>          self.assertIsNot(
>              None, pattern.search(normalise_whitespace(text)), text)
>  
> -    def assertIsInstance(self, instance, assert_class):
> +    def assertIsInstance(self, instance, assert_class, msg=None):

That's fair enough, but in that case at least make it override the standard message if passed.

>          """Assert that an instance is an instance of assert_class.
>  
>          instance and assert_class have the same semantics as the parameters


-- 
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.