Code review comment for ~tsimonq2/ssh-import-id/+git/add-team-support:master

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). http://paste.ubuntu.com/p/2sMhtw7K2d/

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

« Back to merge proposal