launchpad-reviewers team mailing list archive

[Guruprasad <mp+483459@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:suppress-artifactory-pfoe-source-conda-archives into launchpad:master.

Commit message:
Suppress PFOE errors for conda and source archives too

Also log the error message at DEBUG level to not trigger an OOPS

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index 75f688f..8de1a48 100644
--- a/lib/lp/archivepublisher/artifactory.py
+++ b/lib/lp/archivepublisher/artifactory.py
@@ -40,6 +40,14 @@ from lp.soyuz.interfaces.publishing import (
     PoolFileOverwriteError,
 )
 
+# The repository types in which PoolFileOverwriteError exceptions
+# are expected and can be ignored.
+IGNORE_PFOES_REPOSITORY_TYPES = (
+    ArchiveRepositoryFormat.CONDA,
+    ArchiveRepositoryFormat.GENERIC,
+    ArchiveRepositoryFormat.PYTHON,
+)
+
 
 def _path_for(
     archive: IArchive,
@@ -357,23 +365,24 @@ class ArtifactoryPoolEntry:
                 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
+                # package and a binary package for certain package types
+                # 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
+                    in IGNORE_PFOES_REPOSITORY_TYPES
                 ):
-                    self.logger.warning(
+                    self.logger.debug(
                         f"Ignoring PoolFileOverwriteError: {error_message} "
-                        "as it is a known limitation for Python packages."
+                        "as it is a known limitation for "
+                        f"{self.archive.repository_format.title} packages."
                     )
                     raise IgnorableArtifactoryPoolFileOverwriteError(
                         error_message
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index 7667ddb..12701ae 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -213,9 +213,9 @@ class TestArtifactoryPool(TestCase):
         self.assertEqual(pool.results.NONE, result)
         self.assertTrue(foo.checkIsFile())
 
-    def test_addFile_exists_overwrite_non_python_archives(self):
-        # PoolFileOverwriteErrors are raised only for non-Python
-        # Artifactory files.
+    def test_addFile_exists_overwrite_non_ignored_archive_types(self):
+        # PoolFileOverwriteErrors are raised only for non-Python, non-Conda,
+        # non-source archives' Artifactory files.
         pool = self.makePool()
         foo = ArtifactoryPoolTestingFile(
             pool=pool,
@@ -232,8 +232,9 @@ class TestArtifactoryPool(TestCase):
 
     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.
+        # for Python, Conda, and source archive Artifactory files. See
+        # XXX comments in the appropriate sources for more details.
+
         pool = self.makePool(repository_format=ArchiveRepositoryFormat.PYTHON)
         foo = ArtifactoryPoolTestingFile(
             pool=pool,
@@ -250,6 +251,48 @@ class TestArtifactoryPool(TestCase):
             IgnorableArtifactoryPoolFileOverwriteError, foo.addToPool
         )
 
+    def test_addFile_exists_overwrite_ignored_for_conda_archives(self):
+        # IgnorableArtifactoryPoolFileOverwriteErrors are raised only
+        # for Python, Conda, and source archive Artifactory files. See
+        # XXX comments in the appropriate sources for more details.
+
+        pool = self.makePool(repository_format=ArchiveRepositoryFormat.CONDA)
+        foo = ArtifactoryPoolTestingFile(
+            pool=pool,
+            source_name="foo",
+            source_version="1.0",
+            filename="foo-1.0.tar.bz2",
+            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_addFile_exists_overwrite_ignored_for_source_archives(self):
+        # IgnorableArtifactoryPoolFileOverwriteErrors are raised only
+        # for Python, Conda, and source archive Artifactory files. See
+        # XXX comments in the appropriate sources for more details.
+
+        pool = self.makePool(repository_format=ArchiveRepositoryFormat.SOURCE)
+        foo = ArtifactoryPoolTestingFile(
+            pool=pool,
+            source_name="foo",
+            source_version="1.0",
+            filename="foo-1.0.tar.gz",
+            release_type=FakeReleaseType.SOURCE,
+            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/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 2085ae3..2214e2e 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -192,9 +192,9 @@ class ArchivePublisherBase:
         # 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.
+        # Python, Conda, source) 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: