launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/destroy-builder-aborted into lp:launchpad.

Commit message:
Remove now-unused BuilderStatus.ABORTED.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/destroy-builder-aborted/+merge/183460

BuilderStatus.ABORTED is no longer used as of launchpad-buildd 115 (in favour of BuildStatus.ABORTED).  Let's get rid of it.
-- 
https://code.launchpad.net/~cjwatson/launchpad/destroy-builder-aborted/+merge/183460
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/destroy-builder-aborted into lp:launchpad.

=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py	2013-09-02 07:13:06 +0000
+++ lib/lp/buildmaster/interactor.py	2013-09-02 13:27:54 +0000
@@ -346,33 +346,6 @@
 
         d = self.slave.status()
 
-        def got_status(status_sentence):
-            """After we get the status, clean if we have to.
-
-            Always return status_sentence.
-            """
-            # Isolate the BuilderStatus string, always the first token in
-            # BuilderSlave.status().
-            status = status_sentence[0]
-
-            # If the cookie test below fails, it will request an abort of the
-            # builder.  This will leave the builder in the aborted state and
-            # with no assigned job, and we should now "clean" the slave which
-            # will reset its state back to IDLE, ready to accept new builds.
-            # This situation is usually caused by a temporary loss of
-            # communications with the slave and the build manager had to reset
-            # the job.
-            if (status == 'BuilderStatus.ABORTED'
-                    and self.builder.currentjob is None):
-                if logger is not None:
-                    logger.info(
-                        "Builder '%s' being cleaned up from ABORTED" %
-                        (self.builder.name,))
-                d = self.cleanSlave()
-                return d.addCallback(lambda ignored: status_sentence)
-            else:
-                return status_sentence
-
         def rescue_slave(status_sentence):
             # If slave is not building nor waiting, it's not in need of
             # rescuing.
@@ -395,7 +368,6 @@
                             (self.builder.name, slave_build_id, reason))
                 return d.addCallback(log_rescue)
 
-        d.addCallback(got_status)
         d.addCallback(rescue_slave)
         return d
 
@@ -602,7 +574,6 @@
                 'BuilderStatus.IDLE': self.updateBuild_IDLE,
                 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
                 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
-                'BuilderStatus.ABORTED': self.updateBuild_ABORTED,
                 'BuilderStatus.WAITING': self.updateBuild_WAITING,
                 }
             status_sentence, status_dict = statuses
@@ -646,22 +617,6 @@
         queueItem.logtail = "Waiting for slave process to be terminated"
         transaction.commit()
 
-    def updateBuild_ABORTED(self, queueItem, status_sentence, status_dict,
-                            logger):
-        """ABORTING process has successfully terminated.
-
-        Clean the builder for another jobs.
-        """
-        d = self.cleanSlave()
-
-        def got_cleaned(ignored):
-            queueItem.builder = None
-            if queueItem.job.status != JobStatus.FAILED:
-                queueItem.job.fail()
-            queueItem.specific_job.jobAborted()
-            transaction.commit()
-        return d.addCallback(got_cleaned)
-
     def extractBuildStatus(self, status_dict):
         """Read build status name.
 

=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py	2013-09-02 06:34:15 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py	2013-09-02 13:27:54 +0000
@@ -81,9 +81,6 @@
     def jobReset():
         """'Job reset' life cycle event, handle as appropriate."""
 
-    def jobAborted():
-        """'Job aborted' life cycle event, handle as appropriate."""
-
     def jobCancel():
         """'Job cancel' life cycle event."""
 

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2013-09-02 06:34:15 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2013-09-02 13:27:54 +0000
@@ -102,10 +102,6 @@
         """See `IBuildFarmJob`."""
         self.build.updateStatus(BuildStatus.NEEDSBUILD)
 
-    def jobAborted(self):
-        """See `IBuildFarmJob`."""
-        self.build.updateStatus(BuildStatus.NEEDSBUILD)
-
     def jobCancel(self):
         """See `IBuildFarmJob`."""
         self.build.updateStatus(BuildStatus.CANCELLED)

=== modified file 'lib/lp/buildmaster/tests/mock_slaves.py'
--- lib/lp/buildmaster/tests/mock_slaves.py	2013-09-02 06:31:24 +0000
+++ lib/lp/buildmaster/tests/mock_slaves.py	2013-09-02 13:27:54 +0000
@@ -6,7 +6,6 @@
 __metaclass__ = type
 
 __all__ = [
-    'AbortedSlave',
     'AbortingSlave',
     'BrokenSlave',
     'BuildingSlave',
@@ -193,18 +192,6 @@
         return defer.succeed(('BuilderStatus.ABORTING', '1-1'))
 
 
-class AbortedSlave(OkSlave):
-    """A mock slave that looks like it's aborted."""
-
-    def clean(self):
-        self.call_log.append('clean')
-        return defer.succeed(None)
-
-    def status(self):
-        self.call_log.append('status')
-        return defer.succeed(('BuilderStatus.ABORTED', '1-1'))
-
-
 class LostBuildingBrokenSlave:
     """A mock slave building bogus Build/BuildQueue IDs that can't be aborted.
 

=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
--- lib/lp/buildmaster/tests/test_interactor.py	2013-08-30 05:49:44 +0000
+++ lib/lp/buildmaster/tests/test_interactor.py	2013-09-02 13:27:54 +0000
@@ -32,7 +32,6 @@
     CorruptBuildCookie,
     )
 from lp.buildmaster.tests.mock_slaves import (
-    AbortedSlave,
     AbortingSlave,
     BrokenSlave,
     BuildingSlave,
@@ -171,32 +170,6 @@
         interactor = BuilderInteractor(builder)
         self.assertEqual(5, interactor.slave.timeout)
 
-    def test_recovery_of_aborted_virtual_slave(self):
-        # If a nonvirtual slave is in the ABORTED state,
-        # rescueBuilderIfLost should clean it if we don't think it's
-        # currently building anything.
-        aborted_slave = AbortedSlave()
-        builder = MockBuilder(virtualized=False, builderok=False)
-        d = BuilderInteractor(builder, aborted_slave).rescueIfLost()
-
-        def check_slave_calls(ignored):
-            self.assertIn('clean', aborted_slave.call_log)
-
-        return d.addCallback(check_slave_calls)
-
-    def test_recovery_of_aborted_nonvirtual_slave(self):
-        # If a nonvirtual slave is in the ABORTED state,
-        # rescueBuilderIfLost should clean it if we don't think it's
-        # currently building anything.
-        aborted_slave = AbortedSlave()
-        builder = MockBuilder(virtualized=False, builderok=False)
-        d = BuilderInteractor(builder, aborted_slave).rescueIfLost()
-
-        def check_slave_calls(ignored):
-            self.assertIn('clean', aborted_slave.call_log)
-
-        return d.addCallback(check_slave_calls)
-
     def test_recover_ok_slave(self):
         # An idle slave is not rescued.
         slave = OkSlave()
@@ -312,10 +285,6 @@
         self.assertStatus(
             AbortingSlave(), builder_status='BuilderStatus.ABORTING')
 
-    def test_slaveStatus_aborted_slave(self):
-        self.assertStatus(
-            AbortedSlave(), builder_status='BuilderStatus.ABORTED')
-
     def test_isAvailable_with_not_builderok(self):
         # isAvailable() is a wrapper around BuilderSlave.status()
         builder = MockBuilder()