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

46fdbac... by Massimiliano Girardi on 2023-05-31

Added test for new login functionality

fcde0db... by Massimiliano Girardi on 2023-05-31

Minor tweaks to naming and docstrings

[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))

Unmerged commits

fcde0db... by Massimiliano Girardi on 2023-05-31

Minor tweaks to naming and docstrings

46fdbac... by Massimiliano Girardi on 2023-05-31

Added test for new login functionality

fb75733... by Massimiliano Girardi on 2023-05-29

Add LP_CREDENTIALS login and documentation

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."""