Code review comment for lp:~jml/bzr/lp-login-oauth

Revision history for this message
John A Meinel (jameinel) wrote :

So... this doesn't address the fact than bzr startup time is probably significantly impacted by loading all of launchpad lib, even when not running lp commands. Why not refactor it the way Qbzr did it with:

class LaunchpadLibCommand(commands.Command):

  def run(self, *args, **kwargs):
    try:
      import launchpadlib
    except ImportError:
      trace.warning('The python module launchpadlib is not available'
                    ' and it is needed to run %s. Please install it and'
                    ' try again.' % (self.__class__.__name__,)) # not quite right
      return 3 # exceptions are error 3, IIRC
    return self._run_lp(*args, **kwargs)

You could even do that as

  try:
    ... import lp_api
  except ImportError

Then things seem to stay relatively isolated, and we don't slow down importing all of zope, and json parsers when running "bzr status".

I do understand the desire to not register a command that cannot be run (and we can't detect that without trying the import). But I think it is better to show the command, and have in the doc string "needs launchpadlib", and then have a clear error and how the user can resolve it at runtime.

Rather than adding overhead to all commands.

review: Needs Fixing

« Back to merge proposal