diff --git a/buildman/jobutil/buildstatus.py b/buildman/jobutil/buildstatus.py index b862c75c6..662dbaa10 100644 --- a/buildman/jobutil/buildstatus.py +++ b/buildman/jobutil/buildstatus.py @@ -76,7 +76,7 @@ class StatusHandler(object): yield From(self._append_log_message(phase, self._build_logs.PHASE, extra_data)) # Update the repository build with the new phase - raise Return(self._build_model.update_phase(self._uuid, phase)) + raise Return(self._build_model.update_phase_then_close(self._uuid, phase)) def __enter__(self): return self._status diff --git a/data/model/build.py b/data/model/build.py index 8d2dc6048..73685aa1a 100644 --- a/data/model/build.py +++ b/data/model/build.py @@ -6,9 +6,9 @@ from peewee import JOIN_LEFT_OUTER import features from data.database import (BuildTriggerService, RepositoryBuildTrigger, Repository, Namespace, User, - RepositoryBuild, BUILD_PHASE, db_for_update, db_random_func) + RepositoryBuild, BUILD_PHASE, db_random_func, UseThenDisconnect) from data.model import (InvalidBuildTriggerException, InvalidRepositoryBuildException, - db_transaction, user as user_model) + db_transaction, user as user_model, config) PRESUMED_DEAD_BUILD_AGE = timedelta(days=15) PHASES_NOT_ALLOWED_TO_CANCEL_FROM = (BUILD_PHASE.PUSHING, BUILD_PHASE.COMPLETE, @@ -153,36 +153,40 @@ def get_pull_robot_name(trigger): return trigger.pull_robot.username -def _get_build_row_for_update(build_uuid): - return db_for_update(RepositoryBuild.select().where(RepositoryBuild.uuid == build_uuid)).get() +def _get_build_row(build_uuid): + return RepositoryBuild.select().where(RepositoryBuild.uuid == build_uuid).get() -def update_phase(build_uuid, phase): +def update_phase_then_close(build_uuid, phase): """ A function to change the phase of a build """ - try: - build = _get_build_row_for_update(build_uuid) - except RepositoryBuild.DoesNotExist: + with UseThenDisconnect(config.app_config): + try: + build = _get_build_row(build_uuid) + except RepositoryBuild.DoesNotExist: return False - # Can't update a cancelled build - if build.phase == BUILD_PHASE.CANCELLED: - return False + # Can't update a cancelled build + if build.phase == BUILD_PHASE.CANCELLED: + return False - build.phase = phase - build.save() - return True + updated = (RepositoryBuild + .update(phase=phase) + .where(RepositoryBuild.id == build.id, RepositoryBuild.phase == build.phase) + .execute()) + + return updated > 0 -def create_cancel_build_in_queue(build, build_queue): +def create_cancel_build_in_queue(build_phase, build_queue_id, build_queue): """ A function to cancel a build before it leaves the queue """ def cancel_build(): cancelled = False - if build.queue_id is not None: - cancelled = build_queue.cancel(build.queue_id) + if build_queue_id is not None: + cancelled = build_queue.cancel(build_queue_id) - if build.phase != BUILD_PHASE.WAITING: + if build_phase != BUILD_PHASE.WAITING: return False return cancelled @@ -190,42 +194,35 @@ def create_cancel_build_in_queue(build, build_queue): return cancel_build -def create_cancel_build_in_manager(build, build_canceller): +def create_cancel_build_in_manager(build_phase, build_uuid, build_canceller): """ A function to cancel the build before it starts to push """ def cancel_build(): - if build.phase in PHASES_NOT_ALLOWED_TO_CANCEL_FROM: + if build_phase in PHASES_NOT_ALLOWED_TO_CANCEL_FROM: return False - return build_canceller.try_cancel_build(build.uuid) + return build_canceller.try_cancel_build(build_uuid) return cancel_build def cancel_repository_build(build, build_queue): - """ This tries to cancel the build returns true if request is successful false if it can't be cancelled """ - with db_transaction(): - from app import build_canceller - from buildman.jobutil.buildjob import BuildJobNotifier - # Reload the build for update. - # We are loading the build for update so checks should be as quick as possible. - try: - build = _get_build_row_for_update(build.uuid) - except RepositoryBuild.DoesNotExist: - return False + """ This tries to cancel the build returns true if request is successful false + if it can't be cancelled """ + from app import build_canceller + from buildman.jobutil.buildjob import BuildJobNotifier - cancel_builds = [create_cancel_build_in_queue(build, build_queue), - create_cancel_build_in_manager(build, build_canceller), ] - original_phase = build.phase - for cancelled in cancel_builds: - if cancelled(): - build.phase = BUILD_PHASE.CANCELLED + cancel_builds = [create_cancel_build_in_queue(build.phase, build.queue_id, build_queue), + create_cancel_build_in_manager(build.phase, build.uuid, build_canceller), ] + for cancelled in cancel_builds: + if cancelled(): + updated = update_phase_then_close(build.uuid, BUILD_PHASE.CANCELLED) + if updated: BuildJobNotifier(build.uuid).send_notification("build_cancelled") - build.save() - return True - build.phase = original_phase - build.save() - return False + + return updated + + return False def get_archivable_build(): diff --git a/test/test_queries.py b/test/test_queries.py index 6c4367e30..9446e70b9 100644 --- a/test/test_queries.py +++ b/test/test_queries.py @@ -2,7 +2,7 @@ import unittest from unittest import TestCase from app import app -from data.model.build import get_repository_build, update_phase, create_repository_build +from data.model.build import get_repository_build, update_phase_then_close, create_repository_build from initdb import setup_database_for_testing, finished_database_for_testing from data import model from data.database import RepositoryBuild, Image, ImageStorage, BUILD_PHASE @@ -83,7 +83,7 @@ class TestUpdatePhase(unittest.TestCase): repo_build = get_repository_build(build.uuid) self.assertEqual(repo_build.phase, BUILD_PHASE.WAITING) - self.assertTrue(update_phase(build.uuid, BUILD_PHASE.COMPLETE)) + self.assertTrue(update_phase_then_close(build.uuid, BUILD_PHASE.COMPLETE)) repo_build = get_repository_build(build.uuid) @@ -91,7 +91,7 @@ class TestUpdatePhase(unittest.TestCase): repo_build.delete_instance() - self.assertFalse(update_phase(repo_build.uuid, BUILD_PHASE.PULLING)) + self.assertFalse(update_phase_then_close(repo_build.uuid, BUILD_PHASE.PULLING)) @staticmethod def create_build(repository):