launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:set-repack-stats-on-job-complete into launchpad:master.

Commit message:
Set repack stats on job completion

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/402389

LP is now receiving confirmation from Turnip at the end of a repack async task that the repack succeeded. We're only sending this confirmation when the job succeedes - don't see the point in doing so when it failed, but I'm open to suggestions of course.
 
MP is also adding an extra GET call back to Turnip to fetch the repack stats synchronously and set them in the LP DB.

Diagram details: https://chat.canonical.com/canonical/pl/3h7smw6rxingm8ytgifhmind7r
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:set-repack-stats-on-job-complete into launchpad:master.

diff --git a/lib/lp/code/interfaces/gitapi.py b/lib/lp/code/interfaces/gitapi.py
index ab11a79..221ecaa 100644
--- a/lib/lp/code/interfaces/gitapi.py
+++ b/lib/lp/code/interfaces/gitapi.py
@@ -118,3 +118,15 @@ class IGitAPI(Interface):
         :param repository_id: The database ID of the repository, provided by
                     translatePath call when repo creation is necessary.
         """
+
+    def updateRepackStats(path):
+        """Update the repack stats for the repository.
+
+        When code hosting completes a repack asynchronously
+        (Celery task), it should call back this method to
+        indicate that the operation completed and that repack stats
+        (loose_object_count, pack_count and date_last_scanned) for the
+        repository should be updated in Launchpad's database.
+
+        :param path: The path to the repository.
+        """
diff --git a/lib/lp/code/interfaces/githosting.py b/lib/lp/code/interfaces/githosting.py
index 11e18eb..2698d22 100644
--- a/lib/lp/code/interfaces/githosting.py
+++ b/lib/lp/code/interfaces/githosting.py
@@ -164,3 +164,11 @@ class IGitHostingClient(Interface):
             service.
         :param logger: An optional logger.
         """
+
+    def fetchRepackStats(path, logger=None):
+        """Update repack stats for a Git repository.
+
+        :param path: Physical path of the new repository on the hosting
+            service.
+        :param logger: An optional logger.
+        """
diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py
index 4f07f44..b0bbfd0 100644
--- a/lib/lp/code/model/githosting.py
+++ b/lib/lp/code/model/githosting.py
@@ -326,6 +326,22 @@ class GitHostingClient:
                 "Failed to repack Git repository %s: %s" %
                 (path, six.text_type(e)))
 
+    def fetchRepackStats(self, path, logger=None):
+        """See `IGitHostingClient`."""
+
+        url = "/repo/%s/repack" % path
+        try:
+            if logger is not None:
+                logger.info(
+                    "Updating repack stats for repository %s" % (
+                        path))
+            return self._get(url)
+        except requests.RequestException as e:
+            if logger is not None:
+                logger.info(
+                    "Failed to update repack stats for repository %s" % (
+                        path))
+
     def collectGarbage(self, path, logger=None):
         """See `IGitHostingClient`."""
 
diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
index 21e728d..ab0af80 100644
--- a/lib/lp/code/xmlrpc/git.py
+++ b/lib/lp/code/xmlrpc/git.py
@@ -46,6 +46,7 @@ from lp.code.interfaces.codehosting import (
     LAUNCHPAD_SERVICES,
     )
 from lp.code.interfaces.gitapi import IGitAPI
+from lp.code.interfaces.githosting import IGitHostingClient
 from lp.code.interfaces.gitjob import IGitRefScanJobSource
 from lp.code.interfaces.gitlookup import (
     IGitLookup,
@@ -745,3 +746,39 @@ class GitAPI(LaunchpadXMLRPCView):
         else:
             logger.info("abortRepoCreation succeeded: %s" % result)
         return result
+
+    def _updateRepackStats(self, requester, path, auth_params):
+        logger = self._getLogger()
+        if requester == LAUNCHPAD_ANONYMOUS:
+            requester = None
+        repository = getUtility(IGitLookup).getByHostingPath(path)
+        if repository is None:
+            raise faults.GitRepositoryNotFound(path)
+
+        if auth_params is not None:
+            verified = self._verifyAuthParams(
+                requester, repository, auth_params)
+            if self._isWritable(requester, repository, verified):
+                # call Turnip to get repack data and set it
+                stats = getUtility(IGitHostingClient).fetchRepackStats(path)
+                removeSecurityProxy(repository).setRepackData(
+                    stats.get('loose_object_count'),
+                    stats.get('pack_count'))
+
+    def updateRepackStats(self, path, auth_params):
+        """See `IGitAPI`."""
+        logger = self._getLogger(auth_params.get("request-id"))
+        requester_id = _get_requester_id(auth_params)
+        logger.info(
+            "Request received: updateRepackStats('%s')", path)
+        try:
+            result = run_with_login(
+                requester_id, self._updateRepackStats,
+                path, auth_params)
+        except Exception as e:
+            result = e
+        if isinstance(result, xmlrpc_client.Fault):
+            logger.error("updateRepackStats failed: %r", result)
+        else:
+            logger.info("updateRepackStats succeeded: %s" % result)
+        return result