Repository :
http://git.fedorahosted.org/cgit/copr.git
On branch : master
---------------------------------------------------------------
commit d5cfb691babe854e459cc478ae4e25cbc29c57f1
Author: Adam Samalik <asamalik(a)redhat.com>
Date: Thu Jul 10 13:59:56 2014 +0200
better and safer deleting of builds
---------------------------------------------------------------
backend/backend/actions.py | 43 ++++++++++++-------
.../coprs_frontend/coprs/logic/builds_logic.py | 12 ++++--
frontend/coprs_frontend/coprs/models.py | 14 ++++++
.../templates/coprs/detail/_builds_forms.html | 2 +-
frontend/coprs_frontend/tests/coprs_test_case.py | 6 ++-
.../test_views/test_coprs_ns/test_coprs_builds.py | 3 +-
6 files changed, 56 insertions(+), 24 deletions(-)
diff --git a/backend/backend/actions.py b/backend/backend/actions.py
index 180e778..0600723 100644
--- a/backend/backend/actions.py
+++ b/backend/backend/actions.py
@@ -36,7 +36,9 @@ class Action(object):
self.event("Removing copr {0}".format(path))
shutil.rmtree(path)
- elif self.data["object_type"] == "build":
+ elif self.data["object_type"] in ["build-succeeded",
+ "build-skipped",
+ "build-failed"]:
self.event("Action delete build")
project = self.data["old_value"]
packages = map(lambda x:
@@ -57,22 +59,31 @@ class Action(object):
self.event("In chroot {0}".format(chroot))
altered = False
+ # We need to delete the files only if they belong
+ # to the build. For example if my build fails and I send
+ # fixed pkg with the same version again, it succeeds and
+ # than I delete the failed, it would delete the succeeded
+ # files as well - that would be wrong.
for pkg in packages:
- pkg_path = os.path.join(path, chroot, pkg)
- if os.path.isdir(pkg_path):
- self.event("Removing build {0}".format(pkg_path))
- shutil.rmtree(pkg_path)
- altered = True
- else:
- self.event("Package {0} dir not found in chroot
{1}"
- .format(pkg, chroot))
-
- if altered:
- self.event("Running createrepo")
- _, _, err = createrepo(os.path.join(path, chroot), self.lock)
- if err.strip():
- self.event(
- "Error making local repo: {0}".format(err))
+ if self.data["object_type"] ==
"build-succeeded" or \
+ (self.data["object_type"] ==
"build-failed" and
+ os.path.exists(os.path.join(path, chroot, pkg,
"fail"))):
+ pkg_path = os.path.join(path, chroot, pkg)
+ if os.path.isdir(pkg_path):
+ self.event("Removing build
{0}".format(pkg_path))
+ shutil.rmtree(pkg_path)
+ altered = True
+ else:
+ self.event(
+ "Package {0} dir not found in chroot {1}"
+ .format(pkg, chroot))
+
+ if altered:
+ self.event("Running createrepo")
+ _, _, err = createrepo(os.path.join(path, chroot),
self.lock)
+ if err.strip():
+ self.event(
+ "Error making local repo:
{0}".format(err))
log_path = os.path.join(
path, chroot,
diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py
b/frontend/coprs_frontend/coprs/logic/builds_logic.py
index 47db86f..625cd66 100644
--- a/frontend/coprs_frontend/coprs/logic/builds_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py
@@ -160,13 +160,16 @@ class BuildsLogic(object):
raise exceptions.InsufficientRightsException(
"You are not allowed to delete this build.")
- if build.state == "running":
+ if not build.deletable:
raise exceptions.ActionInProgressException(
- "You can not delete build which is running.", "Running
build")
+ "You can not delete build which is not finished.",
+ "Unfinished build")
- if build.state != "failed":
+ # Only failed (and finished), succeeded, skipped and cancelled get here.
+ if build.state != "cancelled": #has nothing in backend to delete
+ object_type = "build-{0}".format(build.state)
action =
models.Action(action_type=helpers.ActionTypeEnum("delete"),
- object_type="build",
+ object_type=object_type,
object_id=build.id,
old_value="{0}/{1}".format(build.copr.owner.name,
build.copr.name),
@@ -174,6 +177,7 @@ class BuildsLogic(object):
created_on=int(time.time()))
db.session.add(action)
+
for build_chroot in build.build_chroots:
db.session.delete(build_chroot)
db.session.delete(build)
diff --git a/frontend/coprs_frontend/coprs/models.py
b/frontend/coprs_frontend/coprs/models.py
index c199902..ffb7009 100644
--- a/frontend/coprs_frontend/coprs/models.py
+++ b/frontend/coprs_frontend/coprs/models.py
@@ -319,6 +319,20 @@ class Build(db.Model, helpers.Serializer):
return self.status != helpers.StatusEnum("pending")
+ @property
+ def deletable(self):
+ """
+ Find out if this build is deletable.
+
+ Build is deletable only when it's finished. (also means cancelled)
+ It is important to remember that "failed" state doesn't ultimately
+ mean it's finished - so we need to check whether the "ended_on"
+ property has been set.
+ """
+
+ return self.state in ["succeeded", "canceled",
"skipped"] or \
+ (self.state == "failed" and self.ended_on is not None)
+
class MockChroot(db.Model, helpers.Serializer):
diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html
b/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html
index 2573643..0fdf872 100644
--- a/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html
+++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_forms.html
@@ -65,7 +65,7 @@
{% endmacro %}
{% macro copr_build_delete_form(build, page) %}
- {% if build.ended_on and build.state != "running" %}
+ {% if build.deletable %}
<form action="{{ url_for('coprs_ns.copr_delete_build',
username=build.copr.owner.name, coprname=build.copr.name, build_id=build.id)}}"
method="post">
<input type="hidden" value="page" value="{{ page
}}">
<input type="submit" value="Delete">
diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py
b/frontend/coprs_frontend/tests/coprs_test_case.py
index 313cef7..179555c 100644
--- a/frontend/coprs_frontend/tests/coprs_test_case.py
+++ b/frontend/coprs_frontend/tests/coprs_test_case.py
@@ -121,10 +121,14 @@ class CoprsTestCase(object):
for build in [self.b1, self.b2, self.b3, self.b4]:
self.db.session.add(build)
+ status = None
+ if build is self.b1: # this build is going to be deleted
+ status = 1 # succeeded
for chroot in build.copr.active_chroots:
buildchroot = models.BuildChroot(
build=build,
- mock_chroot=chroot)
+ mock_chroot=chroot,
+ status = status)
self.db.session.add(buildchroot)
diff --git a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
index dab7a94..cfd2bae 100644
--- a/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
+++ b/frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
@@ -110,14 +110,13 @@ class TestCoprDeleteBuild(CoprsTestCase):
.format(self.u1.name, self.c1.name, self.b1.id),
data={},
follow_redirects=True)
-
assert "Build was deleted" in r.data
b = (self.models.Build.query.filter(
self.models.Build.id == self.b1.id)
.first())
assert b is None
act = self.models.Action.query.first()
- assert act.object_type == "build"
+ assert act.object_type == "build-succeeded"
assert act.old_value == "user1/foocopr"
assert act.data == pkgs