Merge ~tsimonq2/ssh-import-id/+git/add-team-support:master into ssh-import-id:master

Proposed by Simon Quigley
Status: Needs review
Proposed branch: ~tsimonq2/ssh-import-id/+git/add-team-support:master
Merge into: ssh-import-id:master
Diff against target: 32 lines (+14/-1)
1 file modified
ssh_import_id/ (+14/-1)
Reviewer Review Type Date Requested Status
Scott Moser Needs Fixing
Review via email:

Commit message

Add team support to the Launchpad interface (LP: #1745538).

Description of the change

This adds team support to the Launchpad interface for ssh-import-id.

This is done in fetch_keys because it's the right middle point between calling the function to fetch the keys and actually doing it.

It logs into Launchpad anonymously, checks if the ID is valid (and errors out if it isn't), and checks to see if it's a team. If it is a team, it recursively gets the SSH key for every member of the team who has an active Launchpad account (which doesn't include other teams, but could be implemented in the future). If it isn't a team, it just does what it normally does.

If it matters, I have signed the Canonical CLA.

Thanks, and I look forward to your feedback.

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

Well, I like the idea of adding group support.

Unfortunately, I don't have a better solution that provides the desired functionality.

Perhaps we could require '--team' or 'lp:team-myteam' or some other way of indicating that the input is a team. Then we could opportunistically import Launchpadlib and take that path for the team. If the import failed give a reasonable error.

What I'm not so happy about in this implementation is:

a.) the existing user case pays a speed penalty
    $ time python3 -m timeit --setup='from launchpadlib.launchpad import Launchpad' --number=3 --repeat=1 'Launchpad.login_anonymously("ssh-import-id", "production", version="devel")'
    3 loops, best of 1: 1.07 sec per loop

b.) added dependency on Launchpadlib is pretty large.
$ apt-cache show python3-launchpadlib | grep Depen
Depends: python3-httplib2 (>= 0.4.0), python3-keyring (>= 0.5), python3-lazr.restfulclient (>= 0.11.2), python3-lazr.uri (>= 1.0.2-4~), python3-simplejson, python3-wadllib, python3:any (>= 3.3.2-2~)

In a standard cloud image, that ends up getting 10 new packages and 2.6M new
disk footprint (in bionic).

c.) version='devel'.
I think this is basically saying "give me the latest api". Which is kind of a guarantee that it will be broken at some point. Using a 'devel' or 'latest' version of anything isn't really acceptable for something that is expected to continue to work for any reasonable amount of time (such as that of an Ubuntu LTS).

What do you think?

review: Needs Fixing
Revision history for this message
Colin Watson (cjwatson) wrote :

On c), the business of freezing Launchpad APIs never really turned out to work very well in practice, and for most things we generally recommend that people just use 'devel'. Note that there is currently no version of the API other than 'devel' that has an advertised EOL date in the future, so if you're going to take a hard line on this then what you're effectively saying is that people can't write tools that use the Launchpad API, which I don't think is reasonable. In particular, the 'account_status' property that this tool relies on is only exported on devel.

Revision history for this message
Scott Moser (smoser) wrote :


I understand that 'devel' is what most (all) tools use for version. But the majority of tools that I've seen that use launchpad are effectively developer scripts. I've written several, if they broke we fix them in version control somewhere and go on with life. If users of ubuntu use 'ssh-import-id mygroup', they expect that to work, and if it regresses then Ubuntu regressed.
It has a dramatically larger scope.

That said, I do understand your point, and what your voice in particular represents here.

If there were only points 'a' and 'b', I'd still suggest we look for another solution.

Am I missing something ? Is it un-advisable for some reason to just read
and then get the list that way?

Revision history for this message
Colin Watson (cjwatson) wrote :

Here's the reason that won't work:

    account_status = exported(
            title=_("The status of this person's account"), required=True,
            readonly=True, vocabulary=AccountStatus),

I understand your concerns, and they do align with why Launchpad originally tried to produce stable API versions. On the other hand, in practice, all the interfaces being used here are in fact extremely stable and I have a hard time imagining us changing them; we would certainly attempt to get an idea of whether they were being used in production first, and at that point we'd discover the uses by ssh-import-id. I wouldn't expect this to break.

You make a reasonable argument for avoiding launchpadlib in this particular case. I think it would be OK to start from

Revision history for this message
Scott Moser (smoser) wrote :


I poked a bit here is a diff from master that I think seems a reasonable start.

Basically, if the +ssh-keys path failed *and* the url was the default url to launchpad ssh keys, then we will go in looking to see if it is a team also.

The fetch_keys_lp_team can be a normal python function that raises exception on error.
We're hoping to get away from things like 'die' in function calls.

Does that seem a reasonable direction to you?

There are plenty of improvements to be made. I can see that we may well want this to return a tuple of "path" and "keys", where "path" is:
So that the code in import_keys could be adjusted to more clearly indicate how this key got there.


Unmerged commits

1ee29bb... by Simon Quigley

Add team support to ssh-import-id-lp.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/ssh_import_id/ b/ssh_import_id/
2index d2f845e..6371c4d 100644
3--- a/ssh_import_id/
4+++ b/ssh_import_id/
5@@ -39,6 +39,7 @@ try:
6 except ImportError:
7 from urllib import quote_plus
9+from launchpadlib.launchpad import Launchpad
11 from .version import VERSION
13@@ -225,7 +226,19 @@ def fetch_keys(proto, username, useragent):
14 Call out to a subcommand to handle the specified protocol and username
15 """
16 if proto == "lp":
17- return fetch_keys_lp(username, useragent)
18+ launchpad = Launchpad.login_anonymously('ssh-import-id', 'production', version='devel')
19+ try:
20+ lpid = launchpad.people[username]
21+ except:
22+ die("Launchpad ID not found.")
23+ if lpid.is_team:
24+ keys = ""
25+ for member in lpid.members_details:
26+ if member.member.account_status == "Active":
27+ keys = keys + "\n" + fetch_keys_lp(, useragent)
28+ return keys
29+ else:
30+ return fetch_keys_lp(username, useragent)
31 elif proto == "gh":
32 return fetch_keys_gh(username, useragent)


People subscribed via source and target branches

to all changes: