launchpad-reviewers team mailing list archive

[Guruprasad <mp+483306@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:ignore-pfoe-artifactory-publishing into launchpad:master.

Commit message:
Suppress PoolFileOverwriteErrors for Artifactory Python packages

These well-known errors are due to limitations in modelling the
relationship between a source and a binary Python package that gets
published on multiple architectures. Till we fix that design issue,
we want to suppress raising these exceptions so as to not flood the OOPS
system.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/483306
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:ignore-pfoe-artifactory-publishing into launchpad:master.

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index 93d8efe..a12b93b 100644
--- a/lib/lp/archivepublisher/artifactory.py
+++ b/lib/lp/archivepublisher/artifactory.py
@@ -35,6 +35,7 @@ from lp.soyuz.interfaces.files import (
 )
 from lp.soyuz.interfaces.publishing import (
     IBinaryPackagePublishingHistory,
+    IgnorableArtifactoryPoolFileOverwriteError,
     NotInPool,
     PoolFileOverwriteError,
 )
@@ -353,9 +354,27 @@ class ArtifactoryPoolEntry:
             file_hash = targetpath.stat().sha1
             sha1 = lfa.content.sha1
             if sha1 != file_hash:
-                raise PoolFileOverwriteError(
-                    "%s != %s for %s" % (sha1, file_hash, targetpath)
-                )
+                error_message = f"{sha1} != {file_hash} for {targetpath}"
+                # XXX 2025-03-24 lgp171188
+                # Due to issues in modelling the relationship between a source
+                # package and a binary package for Python packages getting
+                # built on multiple architectures, we ended up with a lot of
+                # these PoolFileOverwriteError exceptions getting raised for
+                # the same affected files in each run of the Artifactory
+                # publisher. This is unnecessary and is flooding the OOPS
+                # system with too many OOPSes for the same issue. So as a
+                # temporary, stop-gap solution, we are suppressing the
+                # PoolFileOverwriteError exception here and instead raise
+                # IgnorableArtifactoryPoolFileOverwriteError which is getting
+                # ignored in the appropriate upper layers.
+                if (
+                    self.archive.repository_format
+                    == ArchiveRepositoryFormat.PYTHON
+                ):
+                    raise IgnorableArtifactoryPoolFileOverwriteError(
+                        error_message
+                    )
+                raise PoolFileOverwriteError(error_message)
             return FileAddActionEnum.NONE
 
         self.debug("Deploying %s", targetpath)
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index a5212b7..7667ddb 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -32,6 +32,7 @@ from lp.soyuz.enums import (
     BinaryPackageFormat,
 )
 from lp.soyuz.interfaces.publishing import (
+    IgnorableArtifactoryPoolFileOverwriteError,
     IPublishingSet,
     PoolFileOverwriteError,
 )
@@ -212,7 +213,9 @@ class TestArtifactoryPool(TestCase):
         self.assertEqual(pool.results.NONE, result)
         self.assertTrue(foo.checkIsFile())
 
-    def test_addFile_exists_overwrite(self):
+    def test_addFile_exists_overwrite_non_python_archives(self):
+        # PoolFileOverwriteErrors are raised only for non-Python
+        # Artifactory files.
         pool = self.makePool()
         foo = ArtifactoryPoolTestingFile(
             pool=pool,
@@ -227,6 +230,26 @@ class TestArtifactoryPool(TestCase):
         foo.pub_file.libraryfile.contents = b"different"
         self.assertRaises(PoolFileOverwriteError, foo.addToPool)
 
+    def test_addFile_exists_overwrite_ignored_for_python_archives(self):
+        # IgnorableArtifactoryPoolFileOverwriteErrors are raised only
+        # for Python Artifactory files. See XXX comments in the appropriate
+        # sources.
+        pool = self.makePool(repository_format=ArchiveRepositoryFormat.PYTHON)
+        foo = ArtifactoryPoolTestingFile(
+            pool=pool,
+            source_name="foo",
+            source_version="1.0",
+            filename="foo-1.0.whl",
+            release_type=FakeReleaseType.BINARY,
+            release_id=1,
+        )
+        foo.addToPool()
+        self.assertTrue(foo.checkIsFile())
+        foo.pub_file.libraryfile.contents = b"different"
+        self.assertRaises(
+            IgnorableArtifactoryPoolFileOverwriteError, foo.addToPool
+        )
+
     def test_removeFile(self):
         pool = self.makePool()
         foo = ArtifactoryPoolTestingFile(
diff --git a/lib/lp/soyuz/interfaces/publishing.py b/lib/lp/soyuz/interfaces/publishing.py
index a481aa3..3f1101a 100644
--- a/lib/lp/soyuz/interfaces/publishing.py
+++ b/lib/lp/soyuz/interfaces/publishing.py
@@ -9,6 +9,7 @@ __all__ = [
     "IBinaryPackagePublishingHistory",
     "IBinaryPackagePublishingHistoryEdit",
     "IBinaryPackagePublishingHistoryPublic",
+    "IgnorableArtifactoryPoolFileOverwriteError",
     "IPublishingEdit",
     "IPublishingSet",
     "ISourcePackagePublishingHistory",
@@ -76,6 +77,19 @@ class PoolFileOverwriteError(Exception):
     """
 
 
+class IgnorableArtifactoryPoolFileOverwriteError(Exception):
+    """Raised for an ignorable attempt to overwrite a file in Artifactory.
+
+    Artifactory publishing model has known limitations in modelling the
+    relationship between a source and a binary package for non-deb
+    packages (for example, Python) published on multiple architectures. This
+    causes too many exceptions to be raised in each publisher run and often
+    floods the OOPS system. Due to this, we need to ignore some of these
+    overwrite attempts and not raise an exception. This exception class is
+    used solely for that purpose.
+    """
+
+
 class MissingSymlinkInPool(Exception):
     """Raised when there is a missing symlink in pool.
 
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 78749c0..2085ae3 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -78,6 +78,7 @@ from lp.soyuz.interfaces.distributionjob import (
 from lp.soyuz.interfaces.publishing import (
     DeletionError,
     IBinaryPackagePublishingHistory,
+    IgnorableArtifactoryPoolFileOverwriteError,
     IPublishingSet,
     ISourcePackagePublishingHistory,
     OverrideError,
@@ -188,6 +189,14 @@ class ArchivePublisherBase:
             with temporary_request_timeline(request):
                 error_utility.raising(sys.exc_info(), request)
             log.error("%s (%s)" % (message, request.oopsid))
+        # XXX lgp171188 2025-03-24
+        # There are some known PoolFileOverwriteError exceptions
+        # in the Artifactory publishing process for non-deb (for example,
+        # Python) packages on multiple architectures that we need to
+        # ignore to avoid flooding the OOPS system. The following
+        # `except` clause is needed for that.
+        except IgnorableArtifactoryPoolFileOverwriteError:
+            pass
         else:
             self.setPublished()