launchpad-reviewers team mailing list archive

[Colin Watson <mp+451066@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:initializedistroseriesjob-parent-str into launchpad:master.

Commit message:
InitializeDistroSeriesJob: Handle receiving parent IDs as strings

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

`InitializeDistroSeriesJob` rows are created via a JavaScript UI, and in practice all the IDs are unfortunately passed as strings because they come from HTML form input.  Convert them to ints so that the job doesn't crash.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:initializedistroseriesjob-parent-str into launchpad:master.

diff --git a/lib/lp/soyuz/model/initializedistroseriesjob.py b/lib/lp/soyuz/model/initializedistroseriesjob.py
index e487713..e6ef349 100644
--- a/lib/lp/soyuz/model/initializedistroseriesjob.py
+++ b/lib/lp/soyuz/model/initializedistroseriesjob.py
@@ -141,7 +141,9 @@ class InitializeDistroSeriesJob(DistributionJobDerived):
         parts += ", parent[overlay?/pockets/components]: "
         parents = []
         for i in range(len(self.overlays)):
-            series = IStore(DistroSeries).get(DistroSeries, self.parents[i])
+            series = IStore(DistroSeries).get(
+                DistroSeries, int(self.parents[i])
+            )
             parents.append(
                 "%s[%s/%s/%s]"
                 % (
diff --git a/lib/lp/soyuz/tests/test_initializedistroseriesjob.py b/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
index dd48340..eabd3e6 100644
--- a/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
+++ b/lib/lp/soyuz/tests/test_initializedistroseriesjob.py
@@ -51,13 +51,13 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
     def test_getOopsVars(self):
         parent = self.factory.makeDistroSeries()
         distroseries = self.factory.makeDistroSeries()
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         vars = job.getOopsVars()
         naked_job = removeSecurityProxy(job)
         self.assertIn(("distribution_id", distroseries.distribution.id), vars)
         self.assertIn(("distroseries_id", distroseries.id), vars)
         self.assertIn(("distribution_job_id", naked_job.context.id), vars)
-        self.assertIn(("parent_distroseries_ids", [parent.id]), vars)
+        self.assertIn(("parent_distroseries_ids", [str(parent.id)]), vars)
 
     def _getJobs(self):
         """Return the pending InitializeDistroSeriesJobs as a list."""
@@ -80,12 +80,12 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         overlay_components = ("main", "universe")
         arches = ("i386", "amd64")
         archindep_archtag = "amd64"
-        packagesets = (packageset1.id, packageset2.id)
+        packagesets = (str(packageset1.id), str(packageset2.id))
         rebuild = False
 
         job = self.job_source.create(
             distroseries,
-            [parent1.id, parent2.id],
+            [str(parent1.id), str(parent2.id)],
             arches,
             archindep_archtag,
             packagesets,
@@ -121,12 +121,12 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         # If there's already a pending InitializeDistroSeriesJob for a
         # DistroSeries, InitializeDistroSeriesJob.create() raises an
         # exception.
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         exception = self.assertRaises(
             InitializationPending,
             self.job_source.create,
             distroseries,
-            [parent.id],
+            [str(parent.id)],
         )
         self.assertEqual(job, exception.job)
 
@@ -136,14 +136,14 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         # If there's already a completed InitializeDistroSeriesJob for a
         # DistroSeries, InitializeDistroSeriesJob.create() raises an
         # exception.
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         job.start()
         job.complete()
         exception = self.assertRaises(
             InitializationCompleted,
             self.job_source.create,
             distroseries,
-            [parent.id],
+            [str(parent.id)],
         )
         self.assertEqual(job, exception.job)
 
@@ -153,10 +153,10 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         # If there's already a failed InitializeDistroSeriesJob for a
         # DistroSeries, InitializeDistroSeriesJob.create() schedules a new
         # job.
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         job.start()
         job.fail()
-        self.job_source.create(distroseries, [parent.id])
+        self.job_source.create(distroseries, [str(parent.id)])
 
     def test_run_with_previous_series_already_set(self):
         # InitializationError is raised if a parent series already exists
@@ -167,7 +167,7 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
             distroseries, parent, initialized=True
         )
 
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         expected_message = (
             "Series {child.name} has already been initialised" "."
         ).format(child=distroseries)
@@ -189,7 +189,7 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
 
         job = self.job_source.create(
             distroseries,
-            [parent.id],
+            [str(parent.id)],
             arches,
             archindep_archtag,
             packagesets,
@@ -205,7 +205,7 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         self.assertEqual(naked_job.archindep_archtag, archindep_archtag)
         self.assertEqual(naked_job.packagesets, packagesets)
         self.assertEqual(naked_job.rebuild, False)
-        self.assertEqual(naked_job.parents, (parent.id,))
+        self.assertEqual(naked_job.parents, (str(parent.id),))
         self.assertEqual(naked_job.overlays, overlays)
         self.assertEqual(naked_job.overlay_pockets, overlay_pockets)
         self.assertEqual(naked_job.overlay_components, overlay_components)
@@ -213,9 +213,9 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
     def test_parent(self):
         parent = self.factory.makeDistroSeries()
         distroseries = self.factory.makeDistroSeries()
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         naked_job = removeSecurityProxy(job)
-        self.assertEqual((parent.id,), naked_job.parents)
+        self.assertEqual((str(parent.id),), naked_job.parents)
 
     def test_get(self):
         # InitializeDistroSeriesJob.get() returns the initialization job for
@@ -224,8 +224,8 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         distroseries = self.factory.makeDistroSeries()
         another_distroseries = self.factory.makeDistroSeries()
         self.assertIs(None, self.job_source.get(distroseries))
-        self.job_source.create(distroseries, [parent.id])
-        self.job_source.create(another_distroseries, [parent.id])
+        self.job_source.create(distroseries, [str(parent.id)])
+        self.job_source.create(another_distroseries, [str(parent.id)])
         job = self.job_source.get(distroseries)
         self.assertIsInstance(job, InitializeDistroSeriesJob)
         self.assertEqual(job.distroseries, distroseries)
@@ -235,14 +235,14 @@ class InitializeDistroSeriesJobTests(TestCaseWithFactory):
         # None when no error description is recorded.
         parent = self.factory.makeDistroSeries()
         distroseries = self.factory.makeDistroSeries()
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         self.assertIs(None, removeSecurityProxy(job).error_description)
 
     def test_error_description_set_when_notifying_about_user_errors(self):
         # error_description is set by notifyUserError().
         parent = self.factory.makeDistroSeries()
         distroseries = self.factory.makeDistroSeries()
-        job = self.job_source.create(distroseries, [parent.id])
+        job = self.job_source.create(distroseries, [str(parent.id)])
         message = "This is an example message."
         job.notifyUserError(InitializationError(message))
         self.assertEqual(message, removeSecurityProxy(job).error_description)
@@ -310,7 +310,7 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
 
     def test_job(self):
         parent, child, test1_packageset_id = create_child(self.factory)
-        job = self.job_source.create(child, [parent.id])
+        job = self.job_source.create(child, [str(parent.id)])
         switch_dbuser("initializedistroseries")
 
         job.run()
@@ -323,7 +323,7 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
         arch = parent.nominatedarchindep.architecturetag
         job = self.job_source.create(
             child,
-            [parent.id],
+            [str(parent.id)],
             packagesets=(test1_packageset_id,),
             arches=(arch,),
             rebuild=True,
@@ -344,7 +344,7 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
         parent, child, test1_packageset_id = create_child(self.factory)
         job = self.job_source.create(
             child,
-            [parent.id],
+            [str(parent.id)],
             archindep_archtag=None,
             packagesets=None,
             arches=None,
@@ -363,7 +363,7 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
         parent, child, test1_packageset_id = create_child(self.factory)
         job = self.job_source.create(
             child,
-            [parent.id],
+            [str(parent.id)],
             archindep_archtag=None,
             packagesets=None,
             arches=None,
@@ -386,7 +386,7 @@ class InitializeDistroSeriesJobTestsWithPackages(TestCaseWithFactory):
         self.setupDas(parent, "powerpc", "hppa")
         job = self.job_source.create(
             child,
-            [parent.id],
+            [str(parent.id)],
             archindep_archtag="amd64",
             packagesets=None,
             arches=None,
@@ -423,7 +423,7 @@ class TestViaCelery(TestCaseWithFactory):
         parent, child, test1 = create_child(self.factory)
         job_source = getUtility(IInitializeDistroSeriesJobSource)
         with block_on_job():
-            job_source.create(child, [parent.id])
+            job_source.create(child, [str(parent.id)])
             transaction.commit()
         child.updatePackageCount()
         self.assertEqual(parent.sourcecount, child.sourcecount)