Merge lp:~mwhudson/launchpad/less-import-requestMirrors-bug-487357 into lp:launchpad

[Michael Hudson-Doyle]

Extend the contract between the code import 'worker' and 'monitor' scripts by specifying that an exit code of 2 means that the import was successful but didn't import any new revisions so there's no need to request a mirror of the import branch.

[Michael Hudson-Doyle (mwhudson)]

Hi,

This branch extends the contract between the code import 'worker' and 'monitor' scripts by specifying that an exit code of 2 means that the import was successful but didn't import any new revisions so there's no need to request a mirror of the import branch. I expect this to substantially reduce the load on the puller, as currently about half of all branches pulled are import branches with no new revisions.

I've had some pre-imp discussion with Tim, Jono and Aaron about this.

Cheers,
mwh

[Paul Hummer (rockstar)]

This all looks good. Thanks for doing this.

review: Approve

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2010-01-29 17:15:31 +0000
+++ lib/lp/code/browser/branch.py	2010-02-01 05:56:15 +0000
@@ -81,7 +81,8 @@
     latest_proposals_for_each_branch)
 from lp.code.enums import (
     BranchLifecycleStatus, BranchType, CodeImportJobState,
-    CodeImportReviewStatus, RevisionControlSystems, UICreatableBranchType)
+    CodeImportResultStatus, CodeImportReviewStatus, RevisionControlSystems,
+    UICreatableBranchType)
 from lp.code.errors import InvalidBranchMergeProposal
 from lp.code.interfaces.branch import (
     BranchCreationForbidden, BranchExists, IBranch,
@@ -509,6 +510,13 @@
         """Return the last 10 CodeImportResults."""
         return list(self.context.code_import.results[:10])
 
+    def iconForCodeImportResultStatus(self, status):
+        """The icon to represent the `CodeImportResultStatus` `status`."""
+        if status in CodeImportResultStatus.successes:
+            return "/@@/yes"
+        else:
+            return "/@@/no"
+
     @property
     def is_svn_import(self):
         """True if an imported branch is a SVN import."""
 
=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2010-01-12 02:07:38 +0000
+++ lib/lp/code/enums.py	2010-02-01 05:56:15 +0000
@@ -799,6 +799,13 @@
         Import job completed successfully.
         """)
 
+    SUCCESS_NOCHANGE = DBItem(110, """
+        Success with no changes
+
+        Import job completed successfully, but there were no new revisions to
+        import.
+        """)
+
     FAILURE = DBItem(200, """
         Failure
 
@@ -857,6 +864,8 @@
         job, or the deletion of a CodeImport which had a running job.
         """)
 
+    successes = [SUCCESS, SUCCESS_NOCHANGE]
+
 
 class CodeReviewVote(DBEnumeratedType):
     """Code Review Votes
 
=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2010-01-13 02:51:30 +0000
+++ lib/lp/code/model/codeimport.py	2010-02-01 05:56:15 +0000
@@ -160,7 +160,8 @@
             "coalesce",
             Select(
                 CodeImportResult.id,
-                And(CodeImportResult.status == CodeImportResultStatus.SUCCESS,
+                And(CodeImportResult.status.is_in(
+                        CodeImportResultStatus.successes),
                     CodeImportResult.code_import == self),
                 order_by=Desc(CodeImportResult.id),
                 limit=1),
 
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/model/codeimportjob.py	2010-02-01 05:56:15 +0000
@@ -287,11 +287,14 @@
         # Only start a new one if the import is still in the REVIEWED state.
         if code_import.review_status == CodeImportReviewStatus.REVIEWED:
             self.newJob(code_import)
-        # If the status was successful, update the date_last_successful and
-        # arrange for the branch to be mirrored.
-        if status == CodeImportResultStatus.SUCCESS:
+        # If the status was successful, update date_last_successful.
+        if status in [CodeImportResultStatus.SUCCESS,
+                      CodeImportResultStatus.SUCCESS_NOCHANGE]:
             naked_import = removeSecurityProxy(code_import)
             naked_import.date_last_successful = result.date_created
+        # If the status was successful and revisions were imported, arrange
+        # for the branch to be mirrored.
+        if status == CodeImportResultStatus.SUCCESS:
             code_import.branch.requestMirror()
         getUtility(ICodeImportEventSet).newFinish(
             code_import, machine)
 
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py	2010-01-15 23:55:45 +0000
+++ lib/lp/code/model/tests/test_codeimport.py	2010-02-01 05:56:15 +0000
@@ -394,11 +394,15 @@
         getUtility(ICodeImportJobWorkflow).finishJob(
             running_job, CodeImportResultStatus.FAILURE, None)
 
-    def succeedImport(self, code_import):
+    def succeedImport(self, code_import,
+                      status=CodeImportResultStatus.SUCCESS):
         """Create if necessary a job for `code_import` and have it succeed."""
+        if status not in CodeImportResultStatus.successes:
+            raise AssertionError(
+                "succeedImport() should be called with a successful status!")
         running_job = self.makeRunningJob(code_import)
         getUtility(ICodeImportJobWorkflow).finishJob(
-            running_job, CodeImportResultStatus.SUCCESS, None)
+            running_job, status, None)
 
     def test_consecutive_failure_count_zero_initially(self):
         # A new code import has a consecutive_failure_count of 0.
@@ -407,7 +411,7 @@
 
     def test_consecutive_failure_count_succeed(self):
         # A code import that has succeeded once has a
-        # consecutive_failure_count of 1.
+        # consecutive_failure_count of 0.
         code_import = self.factory.makeCodeImport()
         self.succeedImport(code_import)
         self.assertEqual(0, code_import.consecutive_failure_count)
@@ -419,6 +423,15 @@
         self.failImport(code_import)
         self.assertEqual(1, code_import.consecutive_failure_count)
 
+    def test_consecutive_failure_count_succeed_succeed_no_changes(self):
+        # A code import that has succeeded then succeeded with no changes has
+        # a consecutive_failure_count of 0.
+        code_import = self.factory.makeCodeImport()
+        self.succeedImport(code_import)
+        self.succeedImport(
+            code_import, CodeImportResultStatus.SUCCESS_NOCHANGE)
+        self.assertEqual(0, code_import.consecutive_failure_count)
+
     def test_consecutive_failure_count_fail_fail(self):
         # A code import that has failed twice has a consecutive_failure_count
         # of 2.
 
=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2010-02-01 05:56:15 +0000
@@ -32,8 +32,7 @@
 from lp.code.interfaces.codeimportevent import ICodeImportEventSet
 from lp.code.interfaces.codeimportjob import (
     ICodeImportJobSet, ICodeImportJobWorkflow)
-from lp.code.interfaces.codeimportresult import (
-    ICodeImportResult, ICodeImportResultSet)
+from lp.code.interfaces.codeimportresult import ICodeImportResult
 from lp.registry.interfaces.person import IPersonSet
 from lp.testing import ANONYMOUS, login, logout, TestCaseWithFactory
 from canonical.launchpad.testing.codeimporthelpers import (
@@ -274,7 +273,7 @@
         self.assertReclaimableJobs([stale_job])
         machine = self.factory.makeCodeImportMachine(set_online=True)
         login(ANONYMOUS)
-        job = getUtility(ICodeImportJobSet).getJobForMachine(
+        getUtility(ICodeImportJobSet).getJobForMachine(
             machine.hostname)
         login_for_code_imports()
         # Now there are no reclaimable jobs.
@@ -456,7 +455,7 @@
         # of the job that was deleted when the CodeImport review status
         # changed from REVIEWED. That is the date_job_started of the most
         # recent CodeImportResult plus the effective update interval.
-        job = getUtility(ICodeImportJobWorkflow).newJob(code_import)
+        getUtility(ICodeImportJobWorkflow).newJob(code_import)
         self.assertSqlAttributeEqualsDate(
             code_import.import_job, 'date_due',
             recent_result.date_job_started + interval)
@@ -690,7 +689,6 @@
     def test_wrongJobState(self):
         # Calling updateHeartbeat with a job whose state is not RUNNING is an
         # error.
-        machine = self.factory.makeCodeImportMachine()
         code_import = self.factory.makeCodeImport()
         job = self.factory.makeCodeImportJob(code_import)
         self.assertFailure(
@@ -740,7 +738,6 @@
 
     def test_wrongJobState(self):
         # Calling finishJob with a job whose state is not RUNNING is an error.
-        machine = self.factory.makeCodeImportMachine()
         code_import = self.factory.makeCodeImport()
         job = self.factory.makeCodeImportJob(code_import)
         self.switchDbUser()
@@ -767,7 +764,6 @@
         # finishJob() creates a new CodeImportJob for the given CodeImport,
         # scheduled appropriately far in the future.
         running_job = self.makeRunningJob()
-        running_job_date_due = running_job.date_due
         code_import = running_job.code_import
         self.switchDbUser()
         getUtility(ICodeImportJobWorkflow).finishJob(
@@ -784,7 +780,6 @@
         # finishJob() creates a new CodeImportJob for the given CodeImport,
         # unless the CodeImport has been suspended or marked invalid.
         running_job = self.makeRunningJob()
-        running_job_date_due = running_job.date_due
         code_import = running_job.code_import
         ddaa = getUtility(IPersonSet).getByEmail(
             '[e-mail]')
@@ -798,9 +793,7 @@
     def test_createsResultObject(self):
         # finishJob() creates a CodeImportResult object for the given import.
         running_job = self.makeRunningJob()
-        running_job_date_due = running_job.date_due
         code_import = running_job.code_import
-        result_set = getUtility(ICodeImportResultSet)
         # Before calling finishJob() there are no CodeImportResults for the
         # given import...
         self.assertEqual(len(list(code_import.results)), 0)
@@ -903,7 +896,6 @@
 
         self.switchDbUser()
         log_data = 'several\nlines\nof\nlog data'
-        log_excerpt = log_data.splitlines()[-1]
         log_alias_id = getUtility(ILibrarianClient).addFile(
            'import_log.txt', len(log_data),
            StringIO.StringIO(log_data), 'text/plain')
@@ -939,7 +931,7 @@
             code_import = job.code_import
             self.assertTrue(code_import.date_last_successful is None)
             getUtility(ICodeImportJobWorkflow).finishJob(job, status, None)
-            if status == CodeImportResultStatus.SUCCESS:
+            if status in CodeImportResultStatus.successes:
                 self.assertTrue(code_import.date_last_successful is not None)
             else:
                 self.assertTrue(code_import.date_last_successful is None)
 
=== modified file 'lib/lp/code/stories/codeimport/xx-codeimport-results.txt'
--- lib/lp/code/stories/codeimport/xx-codeimport-results.txt	2009-03-24 12:43:49 +0000
+++ lib/lp/code/stories/codeimport/xx-codeimport-results.txt	2010-02-01 05:56:15 +0000
@@ -23,6 +23,8 @@
     >>> browser.open(branch_url)
     >>> import_results = find_tag_by_id(browser.contents, 'import-results')
     >>> print extract_text(import_results).replace('—', '--')
+    Import started on 2007-12-10 on odin and finished on 2007-12-10
+      taking ten hours -- see the log
     Import started on 2007-12-09 on odin and finished on 2007-12-09
       taking nine hours -- see the log
     Import started on 2007-12-08 on odin and finished on 2007-12-08
@@ -58,4 +60,5 @@
     <img src="/@@/no" title="Source Checkout Failed" />
     <img src="/@@/no" title="Internal Failure" />
     <img src="/@@/no" title="Failure" />
+    <img src="/@@/yes" title="Success with no changes" />
     <img src="/@@/yes" title="Success" />
 
=== modified file 'lib/lp/code/templates/codeimport-macros.pt'
--- lib/lp/code/templates/codeimport-macros.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/code/templates/codeimport-macros.pt	2010-02-01 05:56:15 +0000
@@ -8,10 +8,8 @@
     Expects a parameter called 'result'.
   </tal:comment>
 
-  <img src="/@@/yes" tal:condition="result/status/enumvalue:SUCCESS"
-       title="Success"/>
-  <img src="/@@/no" tal:condition="not: result/status/enumvalue:SUCCESS"
-       tal:attributes="title result/status/title"/>
+  <img tal:attributes="src python:view.iconForCodeImportResultStatus(result.status);
+                       title result/status/title"/>
   Import started
   <tal:when replace="result/date_job_started/fmt:displaydate">
     4 hours ago
 
=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2010-01-13 02:09:34 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2010-02-01 05:56:15 +0000
@@ -30,9 +30,9 @@
 
 from lp.codehosting import load_optional_plugin
 from lp.codehosting.codeimport.worker import (
-    BazaarBranchStore, BzrSvnImportWorker, CSCVSImportWorker,
+    BazaarBranchStore, BzrSvnImportWorker, CodeImportWorkerExitCode,
     ForeignTreeStore, GitImportWorker, HgImportWorker, ImportDataStore,
-    ImportWorker, get_default_bazaar_branch_store)
+    ImportWorker, CSCVSImportWorker, get_default_bazaar_branch_store)
 from lp.codehosting.codeimport.tests.servers import (
     CVSServer, GitServer, MercurialServer, SubversionServer)
 from lp.codehosting.tests.helpers import (
@@ -710,6 +710,28 @@
         self.assertEqual(
             self.foreign_commit_count, len(branch.revision_history()))
 
+    def test_script_exit_codes(self):
+        # After a successful import that imports revisions, the worker exits
+        # with a code of CodeImportWorkerExitCode.SUCCESS.  After a successful
+        # import that does not import revisions, the worker exits with a code
+        # of CodeImportWorkerExitCode.SUCCESS_NOCHANGE.
+        source_details = self.makeSourceDetails(
+            'trunk', [('README', 'Original contents')])
+
+        clean_up_default_stores_for_import(source_details)
+
+        script_path = os.path.join(
+            config.root, 'scripts', 'code-import-worker.py')
+        output = tempfile.TemporaryFile()
+        retcode = subprocess.call(
+            [script_path] + source_details.asArguments(),
+            stderr=output, stdout=output)
+        self.assertEqual(retcode, CodeImportWorkerExitCode.SUCCESS)
+        retcode = subprocess.call(
+            [script_path] + source_details.asArguments(),
+            stderr=output, stdout=output)
+        self.assertEqual(retcode, CodeImportWorkerExitCode.SUCCESS_NOCHANGE)
+
 
 class CSCVSActualImportMixin(TestActualImportMixin):
 
 
=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2010-01-13 02:09:34 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2010-02-01 05:56:15 +0000
@@ -38,7 +38,8 @@
 from lp.code.model.codeimportjob import CodeImportJob
 from lp.codehosting import load_optional_plugin
 from lp.codehosting.codeimport.worker import (
-    CodeImportSourceDetails, get_default_bazaar_branch_store)
+    CodeImportSourceDetails, CodeImportWorkerExitCode,
+    get_default_bazaar_branch_store)
 from lp.codehosting.codeimport.workermonitor import (
     CodeImportWorkerMonitor, CodeImportWorkerMonitorProtocol, ExitQuietly,
     read_only_transaction)
@@ -292,7 +293,39 @@
 
     def test_callFinishJobCallsFinishJobFailure(self):
         # callFinishJob calls finishJob with CodeImportResultStatus.FAILURE
-        # and swallows the failure if its argument is a Failure.
+        # and swallows the failure if its argument indicates that the
+        # subprocess exited with an exit code of
+        # CodeImportWorkerExitCode.FAILURE.
+        calls = self.patchOutFinishJob()
+        ret = self.worker_monitor.callFinishJob(
+            makeFailure(
+                error.ProcessTerminated,
+                exitCode=CodeImportWorkerExitCode.FAILURE))
+        self.assertEqual(calls, [CodeImportResultStatus.FAILURE])
+        self.assertOopsesLogged([error.ProcessTerminated])
+        # We return the deferred that callFinishJob returns -- if
+        # callFinishJob did not swallow the error, this will fail the test.
+        return ret
+
+    def test_callFinishJobCallsFinishJobSuccessNoChange(self):
+        # If the argument to callFinishJob indicates that the subprocess
+        # exited with a code of CodeImportWorkerExitCode.SUCCESS_NOCHANGE, it
+        # calls finishJob with a status of SUCCESS_NOCHANGE.
+        calls = self.patchOutFinishJob()
+        ret = self.worker_monitor.callFinishJob(
+            makeFailure(
+                error.ProcessTerminated,
+                exitCode=CodeImportWorkerExitCode.SUCCESS_NOCHANGE))
+        self.assertEqual(calls, [CodeImportResultStatus.SUCCESS_NOCHANGE])
+        self.assertOopsesLogged([])
+        # We return the deferred that callFinishJob returns -- if
+        # callFinishJob did not swallow the error, this will fail the test.
+        return ret
+
+    def test_callFinishJobCallsFinishJobArbitraryFailure(self):
+        # If the argument to callFinishJob indicates that there was some other
+        # failure that had nothing to do with the subprocess, it records
+        # failure.
         calls = self.patchOutFinishJob()
         ret = self.worker_monitor.callFinishJob(makeFailure(RuntimeError))
         self.assertEqual(calls, [CodeImportResultStatus.FAILURE])
 
=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2010-01-13 02:09:34 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2010-02-01 05:56:15 +0000
@@ -9,6 +9,7 @@
     'BzrSvnImportWorker',
     'CSCVSImportWorker',
     'CodeImportSourceDetails',
+    'CodeImportWorkerExitCode',
     'ForeignTreeStore',
     'GitImportWorker',
     'HgImportWorker',
@@ -29,13 +30,13 @@
 from bzrlib.upgrade import upgrade
 
 from canonical.cachedproperty import cachedproperty
+from lp.code.enums import RevisionControlSystems
 from lp.codehosting.codeimport.foreigntree import (
     CVSWorkingTree, SubversionWorkingTree)
 from lp.codehosting.codeimport.tarball import (
     create_tarball, extract_tarball)
 from lp.codehosting.codeimport.uifactory import LoggingUIFactory
 from canonical.config import config
-from lp.code.enums import RevisionControlSystems
 
 from cscvs.cmds import totla
 import cscvs
@@ -43,6 +44,14 @@
 import SCM
 
 
+class CodeImportWorkerExitCode:
+    """Exit codes used by the code import worker script."""
+
+    SUCCESS = 0
+    FAILURE = 1
+    SUCCESS_NOCHANGE = 2
+
+
 class BazaarBranchStore:
     """A place where Bazaar branches of code imports are kept."""
 
@@ -77,7 +86,11 @@
         return BzrDir.open(target_path).open_workingtree()
 
     def push(self, db_branch_id, bzr_tree, required_format):
-        """Push up `bzr_tree` as the Bazaar branch for `code_import`."""
+        """Push up `bzr_tree` as the Bazaar branch for `code_import`.
+
+        :return: A boolean that is true if the push was non-trivial
+            (i.e. actually transferred revisions).
+        """
         self.transport.create_prefix()
         branch_from = bzr_tree.branch
         target_url = self._getMirrorURL(db_branch_id)
@@ -86,7 +99,8 @@
         except NotBranchError:
             branch_to = BzrDir.create_branch_and_repo(
                 target_url, format=required_format)
-        branch_to.pull(branch_from, overwrite=True)
+        pull_result = branch_to.pull(branch_from, overwrite=True)
+        return pull_result.old_revid != pull_result.new_revid
 
 
 def get_default_bazaar_branch_store():
@@ -356,8 +370,11 @@
             self.required_format)
 
     def pushBazaarWorkingTree(self, bazaar_tree):
-        """Push the updated Bazaar working tree to the server."""
-        self.bazaar_branch_store.push(
+        """Push the updated Bazaar working tree to the server.
+
+        :return: True if revisions were transferred.
+        """
+        return self.bazaar_branch_store.push(
             self.source_details.branch_id, bazaar_tree, self.required_format)
 
     def getWorkingDirectory(self):
@@ -390,12 +407,20 @@
         saved_pwd = os.getcwd()
         os.chdir(working_directory)
         try:
-            self._doImport()
+            non_trivial = self._doImport()
+            if non_trivial:
+                return CodeImportWorkerExitCode.SUCCESS
+            else:
+                return CodeImportWorkerExitCode.SUCCESS_NOCHANGE
         finally:
             shutil.rmtree(working_directory)
             os.chdir(saved_pwd)
 
     def _doImport(self):
+        """Perform the import.
+
+        :return: True if the import actually imported some new revisions.
+        """
         raise NotImplementedError()
 
 
@@ -470,8 +495,9 @@
         foreign_tree = self.getForeignTree()
         bazaar_tree = self.getBazaarWorkingTree()
         self.importToBazaar(foreign_tree, bazaar_tree)
-        self.pushBazaarWorkingTree(bazaar_tree)
+        non_trivial = self.pushBazaarWorkingTree(bazaar_tree)
         self.foreign_tree_store.archive(foreign_tree)
+        return non_trivial
 
 
 class PullingImportWorker(ImportWorker):
@@ -506,7 +532,7 @@
             bazaar_tree.branch.pull(foreign_branch, overwrite=True)
         finally:
             bzrlib.ui.ui_factory = saved_factory
-        self.pushBazaarWorkingTree(bazaar_tree)
+        return self.pushBazaarWorkingTree(bazaar_tree)
 
 
 class GitImportWorker(PullingImportWorker):
@@ -542,9 +568,11 @@
         that bzr-git will have created at .bzr/repository/bzr.git into the
         import data store.
         """
-        PullingImportWorker.pushBazaarWorkingTree(self, bazaar_tree)
+        non_trivial = PullingImportWorker.pushBazaarWorkingTree(
+            self, bazaar_tree)
         self.import_data_store.put(
             'git.db', bazaar_tree.branch.repository._transport)
+        return non_trivial
 
 
 class HgImportWorker(PullingImportWorker):
@@ -581,9 +609,11 @@
         that bzr-hg will have created at .bzr/repository/hg-v2.db into the
         import data store.
         """
-        PullingImportWorker.pushBazaarWorkingTree(self, bazaar_tree)
+        non_trivial = PullingImportWorker.pushBazaarWorkingTree(
+            self, bazaar_tree)
         self.import_data_store.put(
             self.db_file, bazaar_tree.branch.repository._transport)
+        return non_trivial
 
 
 class BzrSvnImportWorker(PullingImportWorker):
 
=== modified file 'lib/lp/codehosting/codeimport/workermonitor.py'
--- lib/lp/codehosting/codeimport/workermonitor.py	2009-06-25 04:06:00 +0000
+++ lib/lp/codehosting/codeimport/workermonitor.py	2010-02-01 05:56:15 +0000
@@ -10,10 +10,9 @@
 
 
 import os
-import sys
 import tempfile
 
-from twisted.internet import defer, reactor, task
+from twisted.internet import defer, error, reactor, task
 from twisted.python import failure
 from twisted.python.util import mergeFunctionMetadata
 
@@ -32,7 +31,8 @@
 from lp.code.enums import CodeImportResultStatus
 from lp.code.interfaces.codeimportjob import (
     ICodeImportJobSet, ICodeImportJobWorkflow)
-from lp.codehosting.codeimport.worker import CodeImportSourceDetails
+from lp.codehosting.codeimport.worker import (
+    CodeImportSourceDetails, CodeImportWorkerExitCode)
 from lp.testing import login, logout, ANONYMOUS
 
 
@@ -243,19 +243,15 @@
     def finishJob(self, status):
         """Call the finishJob method for the job we are working on.
 
-        This method uploads the log file to the librarian first.  If this
-        fails, we still try to call finishJob, but return the librarian's
-        failure if finishJob succeeded (if finishJob fails, that exception
-        'wins').
+        This method uploads the log file to the librarian first.
         """
         job = self.getJob()
         log_file_size = self._log_file.tell()
-        librarian_failure = None
         if log_file_size > 0:
             self._log_file.seek(0)
             branch = job.code_import.branch
-            log_file_name = '%s-%s-log.txt' % (
-                branch.product.name, branch.name)
+            log_file_name = '%s-%s-%s-log.txt' % (
+                branch.owner.name, branch.product.name, branch.name)
             try:
                 log_file_alias = self._createLibrarianFileAlias(
                     log_file_name, log_file_size, self._log_file,
@@ -300,16 +296,25 @@
         failure.trap(ExitQuietly)
         return None
 
+    def _reasonToStatus(self, reason):
+        if isinstance(reason, failure.Failure):
+            if reason.check(error.ProcessTerminated):
+                if reason.value.exitCode == \
+                       CodeImportWorkerExitCode.SUCCESS_NOCHANGE:
+                    return CodeImportResultStatus.SUCCESS_NOCHANGE
+            return CodeImportResultStatus.FAILURE
+        else:
+            return CodeImportResultStatus.SUCCESS
+
     def callFinishJob(self, reason):
         """Call finishJob() with the appropriate status."""
         if not self._call_finish_job:
             return reason
-        if isinstance(reason, failure.Failure):
+        status = self._reasonToStatus(reason)
+        if status == CodeImportResultStatus.FAILURE:
             self._log_file.write("Import failed:\n")
             reason.printTraceback(self._log_file)
             self._logOopsFromFailure(reason)
-            status = CodeImportResultStatus.FAILURE
         else:
             self._logger.info('Import succeeded.')
-            status = CodeImportResultStatus.SUCCESS
         return self.finishJob(status)
 
=== modified file 'scripts/code-import-worker.py'
--- scripts/code-import-worker.py	2009-11-20 07:09:54 +0000
+++ scripts/code-import-worker.py	2010-02-01 05:56:15 +0000
@@ -19,6 +19,7 @@
 import _pythonpath
 
 from optparse import OptionParser
+import sys
 
 from bzrlib.transport import get_transport
 
@@ -58,9 +59,9 @@
             source_details,
             get_transport(config.codeimport.foreign_tree_store),
             get_default_bazaar_branch_store(), self.logger)
-        import_worker.run()
+        return import_worker.run()
 
 
 if __name__ == '__main__':
     script = CodeImportWorker()
-    script.main()
+    sys.exit(script.main())