Launchpad itself

Merge ~xnox/launchpad:fixup-i18n-index-publish into launchpad:master

  1. Git
  2. lp:~xnox/launchpad
  3. fixup-i18n-index-publish
  4. Merge into master
Proposed by Dimitri John Ledkov
Status: Merged
Approved by: Guruprasad
Approved revision: 2172d477047815bc9d59e14f4f592b3d171654ee
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~xnox/launchpad:fixup-i18n-index-publish
Merge into: launchpad:master
Diff against target: 61 lines (+25/-2)
2 files modified
lib/lp/archivepublisher/publishing.py (+5/-1)
lib/lp/archivepublisher/tests/test_publisher.py (+20/-1)
Reviewer Review Type Date Requested Status
Dimitri John Ledkov (community) Approve
William Grant code Approve
Guruprasad Approve
Review via email: mp+458162@code.launchpad.net

Commit message

soyuz: fix up removal of stale i18n/Index upon removal

During qastaging testing it was discovered that i18n/Index publishing
correctly honors publish_i18n_index setting for freshly created suites
in an archive. But existing archives incorrectly left stale i18n/Index
on disk (whilst scheduling byhash symlink removal).

QA Testing:
* create a new archive on qastaging
* publish it once
* verify i18n/Index is published with byhash link to it
* toggle publish_i18n_index off
* publish it again
* verify i18n/Index is gone straight away
* verify deathrow 24h later removed byhash link to it as well

Fixes: 5f90943b2f ("soyuz: Add toggle to turn off I18n/Index publishing")

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

LGTM 👍 As discussed already, I will request a review + approval from William as well before we merge this.

review: Approve
Revision history for this message
Dimitri John Ledkov (xnox) :
Revision history for this message
William Grant (wgrant) :
review: Approve (code)
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

And had to fix up the test case further - as it was not setting the publish_i18n_index flag on the wrong distro series. And the test cases worked before, because index is never published if it references no translation files. Thus fixed up test case to add empty Translation-en to cause index generation; and flipping the publish_i18n_index flag on the right distro series.

So this actually tests the flag properly now, including flipping the state of it.

This now also addresses wgrant review comments above.

review: Approve
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

rebased and squashed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/archivepublisher/publishing.py b/lib/lp/archivepublisher/publishing.py
index 165c030..bddf01e 100644
--- a/lib/lp/archivepublisher/publishing.py
+++ b/lib/lp/archivepublisher/publishing.py
@@ -1682,12 +1682,16 @@ class Publisher:
1682 # Schedule i18n files for inclusion in the Release file.1682 # Schedule i18n files for inclusion in the Release file.
1683 all_series_files.add(os.path.join(i18n_subpath, i18n_file))1683 all_series_files.add(os.path.join(i18n_subpath, i18n_file))
16841684
1685 i18n_file_name = os.path.join(i18n_dir, "Index")
1685 if distroseries.publish_i18n_index:1686 if distroseries.publish_i18n_index:
1686 with open(os.path.join(i18n_dir, "Index"), "wb") as f:1687 with open(i18n_file_name, "wb") as f:
1687 i18n_index.dump(f, "utf-8")1688 i18n_index.dump(f, "utf-8")
16881689
1689 # Schedule this for inclusion in the Release file.1690 # Schedule this for inclusion in the Release file.
1690 all_series_files.add(os.path.join(component, "i18n", "Index"))1691 all_series_files.add(os.path.join(component, "i18n", "Index"))
1692 else:
1693 if os.path.exists(i18n_file_name):
1694 os.unlink(i18n_file_name)
16911695
1692 def _readIndexFileHashes(1696 def _readIndexFileHashes(
1693 self, suite, file_name, subpath=None, real_file_name=None1697 self, suite, file_name, subpath=None, real_file_name=None
diff --git a/lib/lp/archivepublisher/tests/test_publisher.py b/lib/lp/archivepublisher/tests/test_publisher.py
index 91a3c0b..70c9c70 100644
--- a/lib/lp/archivepublisher/tests/test_publisher.py
+++ b/lib/lp/archivepublisher/tests/test_publisher.py
@@ -2859,7 +2859,25 @@ class TestPublisher(TestPublisherBase):
2859 self.config.distsroot, "breezy-autotest", "main", "i18n"2859 self.config.distsroot, "breezy-autotest", "main", "i18n"
2860 )2860 )
28612861
2862 self.ubuntu["breezy-autotest"].publish_i18n_index = False2862 # Write compressed versions of a zero-length Translation-en file.
2863 translation_en_index = RepositoryIndexFile(
2864 os.path.join(i18n_root, "Translation-en"),
2865 self.config.temproot,
2866 self.ubuntutest["breezy-autotest"].index_compressors,
2867 )
2868 translation_en_index.close()
2869
2870 # First publish i18n/Index (status quo)
2871 publisher._writeSuiteI18n(
2872 self.ubuntutest["breezy-autotest"],
2873 PackagePublishingPocket.RELEASE,
2874 "main",
2875 set(),
2876 )
2877
2878 self.assertTrue(os.path.exists(os.path.join(i18n_root, "Index")))
2879
2880 self.ubuntutest["breezy-autotest"].publish_i18n_index = False
28632881
2864 publisher._writeSuiteI18n(2882 publisher._writeSuiteI18n(
2865 self.ubuntutest["breezy-autotest"],2883 self.ubuntutest["breezy-autotest"],
@@ -2868,6 +2886,7 @@ class TestPublisher(TestPublisherBase):
2868 set(),2886 set(),
2869 )2887 )
28702888
2889 # Ensure it is removed, if previously existed
2871 self.assertFalse(os.path.exists(os.path.join(i18n_root, "Index")))2890 self.assertFalse(os.path.exists(os.path.join(i18n_root, "Index")))
28722891
2873 def testReadIndexFileHashesCompression(self):2892 def testReadIndexFileHashesCompression(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: