Merge ~hook25/ppa-dev-tools:support_env_credentials into ppa-dev-tools:main

[Massimiliano Girardi]

Add LP_CREDENTIALS login and documentation

This adds the possibility of running ppa-dev-tools automatically by making it retrieve the credentials for Launchpad from the LP_CREDENTIALS environment variable and documents another way to do so via the LP_CREDENTIALS_FILE environment variable

[Bryce Harrington (bryce)]

Good addition, a few nitpicks below but otherwise LGTM.

Other than a couple minor flake8 issues, `make check` passes.

Would you be willing to write a test case to go along with this? We have test_new_connection() that verifies that the login_with() call gets invoked, what we'd need is something analogous to that but that sets the environment variable and verifies that Credentials.from_string() gets invoked instead, and login_with() does NOT get invoked.

review: Needs Fixing

[Massimiliano Girardi (hook25)]

I have a few replies to your comments and implemented your feedback where it was necessary (namely, new test, fixed naming of internal functions, removed useless proxy to (envvar or login))

[Bryce Harrington (bryce)]

Looks great, I've squashed the updates and pushed to the main branch:

To git+ssh://git.launchpad.net/ppa-dev-tools
   9dbaa6f..33b9284 main -> main

review: Approve

diff --git a/ppa/lp.py b/ppa/lp.py
index 17b225a..9149810 100644
--- a/ppa/lp.py
+++ b/ppa/lp.py
@@ -14,11 +14,12 @@
 High level wrapper object for Launchpad's API
 """
 
+import os
 from contextlib import suppress
 from functools import lru_cache
 
 from launchpadlib.launchpad import Launchpad
-
+from launchpadlib.credentials import Credentials
 
 class Lp:
     """
@@ -52,8 +53,38 @@ class Lp:
         else:
             self._service_root = 'production'
 
-    def _get_instance(self):
-        """Authenticate to Launchpad."""
+    def _get_instance_from_env(self) -> 'launchpadlib.launchpad.Launchpad | None':
+        """
+        Get an instance of _service using `$LP_CREDENTIAL` environment
+        variable if it is defined, else return None
+
+        :rtype: launchpadlib.launchpad.Launchpad | None
+        :returns: Logged in Launchpad instance if LP_CREDENTIALS is in env
+            else None
+        """
+        lp_cred = os.getenv("LP_CREDENTIALS")
+        if lp_cred:
+            cred = Credentials.from_string(lp_cred)
+            return self._service(
+                cred, None, None,
+                service_root=self._service_root,
+                version='devel'
+            )
+        return None
+
+    def _get_instance_from_login(self) -> 'launchpadlib.launchpad.Launchpad':
+        """
+        Prompts the user to authorize the login of a new credential
+        or use the cached one if it is available and valid
+
+        Note: This uses Launchpad.login_with if service is left to
+            default, if you have a credential file you can provide
+            the path to it via the LP_CREDENTIALS_FILE environment
+            variable and this call will pass without prompting the
+            user
+        :rtype: launchpadlib.launchpad.Launchpad
+        :returns: Logged in Launchpad instance
+        """
         return self._service.login_with(
             application_name=self._app_name,
             service_root=self._service_root,
@@ -65,7 +96,9 @@ class Lp:
     def _instance(self):
         """Cache LP object."""
         if not self._real_instance:
-            self._real_instance = self._get_instance()
+            self._real_instance = (
+                self._get_instance_from_env() or self._get_instance_from_login()
+            )
         return self._real_instance
 
     @property
diff --git a/tests/test_lp.py b/tests/test_lp.py
index 4fe1410..a0c1546 100644
--- a/tests/test_lp.py
+++ b/tests/test_lp.py
@@ -34,7 +34,7 @@ import os.path
 
 import logging
 import pytest
-from mock import Mock
+from mock import Mock, patch
 from launchpadlib.launchpad import Launchpad
 
 sys.path.insert(0, os.path.realpath(
@@ -75,6 +75,33 @@ def test_new_connection():
         version='devel',
     )
 
[e-mail](os.environ, {"LP_CREDENTIALS" : "..."})
+@patch("ppa.lp.Credentials")
+def test_new_connection_envvar(credentials_mock):
+    """Verifies the Lp object will login via envvar if provided."""
+    mock_launchpad = Mock(spec=Launchpad)
+
+    lp = Lp(
+        application_name=APPNAME,
+        service=mock_launchpad)
+
+    # Cause the lp._instance() internal property to be triggered
+    logging.debug(lp.me)
+    logging.debug(lp.people)
+
+    # Verify the triggering did not result in a login attempt
+    mock_launchpad.login_with.assert_not_called()
+    # Rather that the login was done via envvar
+    # creating the credentials
+    credentials_mock.from_string.assert_called_once()
+    # and the service was created with the given credentials
+    mock_launchpad.assert_called_once_with(
+        credentials_mock.from_string.return_value,
+        None,
+        None,
+        service_root="production",
+        version="devel"
+    )
 
 def test_api_root(fakelp):
     """Ensures we can get LP's API root."""