From d8720ff85803bf950f0f06fe3b61971467877d82 Mon Sep 17 00:00:00 2001 From: Dominik Turecek Date: May 11 2020 10:04:58 +0000 Subject: cli, frontend, python: enable deleting multiple builds from cli --- diff --git a/cli/copr_cli/main.py b/cli/copr_cli/main.py index 0fbc30a..f549ee8 100644 --- a/cli/copr_cli/main.py +++ b/cli/copr_cli/main.py @@ -500,8 +500,8 @@ class Commands(object): self._watch_builds(args.build_id) def action_delete_build(self, args): - build = self.client.build_proxy.delete(args.build_id) - print("Build deleted") + result = self.client.build_proxy.delete_list(args.build_id) + print("Build(s) {0} were deleted.".format(", ".join(map(str, result["builds"])))) ######################################################### ### Chroot actions ### @@ -1039,11 +1039,10 @@ def setup_parser(): # create the parser for the "delete-build" command parser_delete = subparsers.add_parser("delete-build", - help="Delete build specified by its ID") - parser_delete.add_argument("build_id", help="Build ID", type=int) + help="Delete builds specified by their IDs") + parser_delete.add_argument("build_id", help="Build ID", type=int, nargs="+") parser_delete.set_defaults(func="action_delete_build") - ######################################################### ### Chroot options ### ######################################################### diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py index 3bee863..7af276a 100644 --- a/frontend/coprs_frontend/coprs/logic/builds_logic.py +++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py @@ -25,6 +25,7 @@ from coprs import models from coprs import helpers from coprs.exceptions import ( ActionInProgressException, + BadRequest, ConflictingRequest, DuplicateException, InsufficientRightsException, @@ -960,15 +961,43 @@ class BuildsLogic(object): db.session.delete(build) @classmethod - def delete_multiple_builds(cls, user, builds): + def delete_builds(cls, user, build_ids): """ + Delete builds specified by list of IDs + :type user: models.User - :type builds: list of models.Build + :type build_ids: list of Int """ to_delete = [] + no_permission = [] + still_running = [] + + build_ids = set(build_ids) + builds = cls.get_by_ids(build_ids) for build in builds: - cls.check_build_to_delete(user, build) - to_delete.append(build) + try: + cls.check_build_to_delete(user, build) + to_delete.append(build) + except InsufficientRightsException: + no_permission.append(build.id) + except ActionInProgressException: + still_running.append(build.id) + finally: + build_ids.remove(build.id) + + if build_ids or no_permission or still_running: + msg = "" + if no_permission: + msg += "You don't have permissions to delete build(s) {0}.\n"\ + .format(", ".join(map(str, no_permission))) + if still_running: + msg += "Build(s) {0} are still running.\n"\ + .format(", ".join(map(str, still_running))) + if build_ids: + msg += "Build(s) {0} don't exist.\n"\ + .format(", ".join(map(str, build_ids))) + + raise BadRequest(msg) if to_delete: ActionsLogic.send_delete_multiple_builds(to_delete) diff --git a/frontend/coprs_frontend/coprs/logic/packages_logic.py b/frontend/coprs_frontend/coprs/logic/packages_logic.py index 780a68d..a49692d 100644 --- a/frontend/coprs_frontend/coprs/logic/packages_logic.py +++ b/frontend/coprs_frontend/coprs/logic/packages_logic.py @@ -191,9 +191,9 @@ class PackagesLogic(object): to_delete = [] for build in package.builds: - to_delete.append(build) + to_delete.append(build.id) - builds_logic.BuildsLogic.delete_multiple_builds(user, to_delete) + builds_logic.BuildsLogic.delete_builds(user, to_delete) db.session.delete(package) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py index f903a64..d489ea1 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py @@ -281,3 +281,15 @@ def delete_build(build_id): BuildsLogic.delete_build(flask.g.user, build) db.session.commit() return flask.jsonify(build_dict) + + +@apiv3_ns.route("/build/delete/list", methods=POST) +@api_login_required +def delete_builds(): + """ + Delete builds specified by a list of IDs. + """ + build_ids = flask.request.json["builds"] + BuildsLogic.delete_builds(flask.g.user, build_ids) + db.session.commit() + return flask.jsonify({"builds": build_ids}) diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index a4263a4..69ae589 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -17,7 +17,8 @@ from coprs.views.coprs_ns import coprs_ns from coprs.exceptions import (ActionInProgressException, InsufficientRightsException, - UnrepeatableBuildException) + UnrepeatableBuildException, + BadRequest) @coprs_ns.route("/build//") @@ -479,12 +480,15 @@ def copr_delete_builds(copr): to_delete = [] for build_id in build_ids: - build = ComplexLogic.get_build_safe(build_id) - to_delete.append(build) + to_delete.append(int(build_id)) - builds_logic.BuildsLogic.delete_multiple_builds(flask.g.user, to_delete) + try: + builds_logic.BuildsLogic.delete_builds(flask.g.user, to_delete) - db.session.commit() - build_ids_str = ", ".join(build_ids).strip(", ") - flask.flash("Builds {} have been deleted successfully.".format(build_ids_str), "success") - return flask.jsonify({"msg": "success"}) + db.session.commit() + build_ids_str = ", ".join(build_ids).strip(", ") + flask.flash("Builds {} have been deleted successfully.".format(build_ids_str), "success") + return flask.jsonify({"msg": "success"}) + except BadRequest as e: + flask.flash(e, "error") + return flask.jsonify({"msg": "error"}) diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py index 24a33f4..f7f223c 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py @@ -9,7 +9,8 @@ from coprs import models from coprs import app from copr_common.enums import StatusEnum -from coprs.exceptions import ActionInProgressException, InsufficientRightsException, MalformedArgumentException +from coprs.exceptions import ActionInProgressException, InsufficientRightsException, \ + MalformedArgumentException, BadRequest from coprs.logic.actions_logic import ActionsLogic from coprs.logic.builds_logic import BuildsLogic @@ -248,8 +249,7 @@ class TestBuildsLogic(CoprsTestCase): self.db.session.commit() assert len(ActionsLogic.get_many().all()) == 0 - BuildsLogic.delete_multiple_builds(self.u1, [self.b1, self.b2, - self.b_pr]) + BuildsLogic.delete_builds(self.u1, [self.b1.id, self.b2.id, self.b_pr.id]) self.db.session.commit() assert len(ActionsLogic.get_many().all()) == 1 @@ -300,6 +300,39 @@ class TestBuildsLogic(CoprsTestCase): with pytest.raises(NoResultFound): BuildsLogic.get(self.b1.id).one() + @pytest.mark.usefixtures("f_users", "f_coprs", "f_builds") + def test_delete_multiple_builds(self): + """ + Test deleting multiple builds at once. + """ + self.b4_bc[0].status = StatusEnum("succeeded") + build_ids = [self.b1.id, self.b2.id, self.b3.id, self.b4.id, 1234] + with pytest.raises(BadRequest) as err_msg: + BuildsLogic.delete_builds(self.u2, build_ids) + + msg1 = "Build(s) {0} are still running".format(self.b3.id) + msg2 = "Build(s) 1234 don't exist" + msg3 = ("You don't have permissions to delete build(s) {0}, {1}" + .format(self.b1.id, self.b2.id)) + + for msg in [msg1, msg2, msg3]: + assert msg in str(err_msg.value) + + for build_id in [self.b1.id, self.b2.id, self.b3.id, self.b4.id]: + BuildsLogic.get(build_id).one() + + self.c1.user = self.u2 + build_ids = [self.b1.id, self.b4.id] + BuildsLogic.delete_builds(self.u2, build_ids) + self.db.session.commit() + + assert len(ActionsLogic.get_many().all()) == 1 + + with pytest.raises(NoResultFound): + BuildsLogic.get(self.b1.id).one() + with pytest.raises(NoResultFound): + BuildsLogic.get(self.b4.id).one() + def test_mark_as_failed(self, f_users, f_coprs, f_mock_chroots, f_builds): self.b1.source_status = StatusEnum("succeeded") self.db.session.commit() diff --git a/python/copr/v3/proxies/build.py b/python/copr/v3/proxies/build.py index 18fc96e..4d7a1c5 100644 --- a/python/copr/v3/proxies/build.py +++ b/python/copr/v3/proxies/build.py @@ -275,5 +275,17 @@ class BuildProxy(BaseProxy): """ endpoint = "/build/delete/{0}".format(build_id) request = Request(endpoint, api_base_url=self.api_base_url, method=POST, auth=self.auth) + + def delete_list(self, build_ids): + """ + Delete multiple builds specified by a list of their ids. + + :param list[int] build_id: + :return: Munch + """ + + endpoint = "/build/delete/list" + data = {"builds": build_ids} + request = Request(endpoint, data=data, api_base_url=self.api_base_url, method=POST, auth=self.auth) response = request.send() return munchify(response)