copr / copr

Clone

#1182 frontend, backend: fix multi-build delete
Merged 4 years ago by praiskup. Opened 4 years ago by praiskup.
Unknown source fix-1180  into  master

file modified
+41 -22 
@@ -264,26 +264,39 @@
 

              except CreateRepoError:
 

                  self.log.exception("Error making local repo: %s", chroot_path)
 

  

-     def delete_build(self, ownername, project_dirname, chroot_builddirs):
 

+     def delete_builddirs(self, ownername, project_dirname, chroot_builddirs):
 

+         """
 

+         delete build (sub)directories from chroot directory, chroot_builddirs
 

+         parameter is in form [{str_chrootname_2: [str_builddir1, str_builddir2]}]
 

+         """
 

          self.log.info("Going to delete: %s", chroot_builddirs)
 

  

-         for chroot, builddir in chroot_builddirs.items():
 

-             if not builddir:
 

-                 self.log.warning("Received empty builddir!")
 

+         for chroot, builddirs in chroot_builddirs.items():
 

+             if not builddirs:
 

+                 self.log.warning("No builddirs received!")
 

                  continue
 

  

-             chroot_path = os.path.join(self.destdir, ownername, project_dirname, chroot)
 

-             builddir_path = os.path.join(chroot_path, builddir)
 

- 

-             if not os.path.isdir(builddir_path):
 

-                 self.log.error("%s not found", builddir_path)
 

-                 continue
 

+             chroot_path = os.path.join(self.destdir, ownername,
 

+                                        project_dirname, chroot)
 

+             for builddir in builddirs:
 

+                 builddir_path = os.path.join(chroot_path, builddir)
 

+                 if not os.path.isdir(builddir_path):
 

+                     self.log.error("%s not found", builddir_path)
 

+                     continue
 

  

-             self.log.debug("builddir to be deleted %s", builddir_path)
 

-             shutil.rmtree(builddir_path)
 

+                 self.log.debug("builddir to be deleted %s", builddir_path)
 

+                 shutil.rmtree(builddir_path)
 

  

      def handle_delete_build(self):
 

          self.log.info("Action delete build.")
 

+ 

+         # == EXAMPLE DATA ==
 

+         # ownername: @copr
 

+         # projectname: TEST1576047114845905086Project10Fork
 

+         # project_dirname: TEST1576047114845905086Project10Fork:pr:12
 

+         # chroot_builddirs:
 

+         #   srpm-builds: [00849545]
 

+         #   fedora-30-x86_64: [00849545-example]
 

          ext_data = json.loads(self.data["data"])
 

  

          ownername = ext_data["ownername"]
 
@@ -292,25 +305,31 @@
 

          chroot_builddirs = ext_data["chroot_builddirs"]
 

          chroots = list(chroot_builddirs.keys())
 

  

-         self.delete_build(ownername, project_dirname, chroot_builddirs)
 

+         self.delete_builddirs(ownername, project_dirname, chroot_builddirs)
 

          self.run_createrepo(ownername, projectname, project_dirname, chroots)
 

  

      def handle_delete_multiple_builds(self):
 

          self.log.debug("Action delete multiple builds.")
 

+ 

+         # == EXAMPLE DATA ==
 

+         # ownername: @copr
 

+         # projectname: testproject
 

+         # project_dirnames:
 

+         #   testproject:pr:10:
 

+         #     srpm-builds: [00849545, 00849546]
 

+         #     fedora-30-x86_64: [00849545-example, 00849545-foo]
 

+         #   [...]
 

          ext_data = json.loads(self.data["data"])
 

  

          ownername = ext_data["ownername"]
 

          projectname = ext_data["projectname"]
 

-         project_dirname = ext_data["project_dirname"]
 

- 

-         chroots = []
 

-         for chroot_builddirs in ext_data["builds"]:
 

-             self.delete_build(ownername, project_dirname, chroot_builddirs)
 

-             for chroot in chroot_builddirs.keys():
 

-                 if chroot not in chroots:
 

-                     chroots.append(chroot)
 

+         project_dirnames = ext_data["project_dirnames"]
 

  

-         self.run_createrepo(ownername, projectname, project_dirname, chroots)
 

+         for project_dirname, chroot_builddirs in project_dirnames.items():
 

+             self.delete_builddirs(ownername, project_dirname, chroot_builddirs)
 

+             chroots = [ch for ch in chroot_builddirs]
 

+             self.run_createrepo(ownername, projectname, project_dirname,
 

+                                 chroots)
 

  

      def handle_delete_chroot(self):
 

          self.log.info("Action delete project chroot.")

file modified
+5 -5 
@@ -606,11 +606,11 @@
 

          ext_data = json.dumps({
 

              "ownername": "foo",
 

              "projectname": "bar",
 

-             "project_dirname": "bar",
 

-             "builds": [
 

-                 {"fedora-20": "01-foo"},
 

-                 {"fedora-20": "02-foo"},
 

-             ]
 

+             "project_dirnames": {
 

+                 'bar': {
 

+                     "fedora-20": ["01-foo", "02-foo"],
 

+                 }
 

+             },
 

          })
 

  

          self.opts.destdir = tmp_dir

file modified
+31 -18 
@@ -119,10 +119,10 @@
 

          Creates a dictionary of chroot builddirs for build delete action
 

          :type build: models.build
 

          """
 

-         chroot_builddirs = {'srpm-builds': build.result_dir}
 

+         chroot_builddirs = {'srpm-builds': [build.result_dir]}
 

  

          for build_chroot in build.build_chroots:
 

-             chroot_builddirs[build_chroot.name] = build_chroot.result_dir
 

+             chroot_builddirs[build_chroot.name] = [build_chroot.result_dir]
 

  

          return chroot_builddirs
 

  
@@ -132,32 +132,25 @@
 

          Creates data needed for build delete action
 

          :type build: models.build
 

          """
 

-         data = {
 

+         return {
 

              "ownername": build.copr.owner_name,
 

              "projectname": build.copr_name,
 

+             "project_dirname":
 

+                 build.copr_dirname if build.copr_dir else build.copr_name,
 

+             "chroot_builddirs": cls.get_chroot_builddirs(build)
 

          }
 

  

-         if build.copr_dir:
 

-             data["project_dirname"] = build.copr_dirname
 

-         else:
 

-             data["project_dirname"] = build.copr_name
 

- 

-         return data
 

- 

      @classmethod
 

      def send_delete_build(cls, build):
 

          """
 

          Schedules build delete action
 

          :type build: models.Build
 

          """
 

-         data = cls.get_build_delete_data(build)
 

-         data["chroot_builddirs"] = cls.get_chroot_builddirs(build)
 

- 

          action = models.Action(
 

              action_type=ActionTypeEnum("delete"),
 

              object_type="build",
 

              object_id=build.id,
 

-             data=json.dumps(data),
 

+             data=json.dumps(cls.get_build_delete_data(build)),
 

              created_on=int(time.time())
 

          )
 

          db.session.add(action)
 
@@ -168,11 +161,31 @@
 

          Schedules builds delete action for builds belonging to the same project
 

          :type build: list of models.Build
 

          """
 

-         data = cls.get_build_delete_data(builds[0])
 

-         data["builds"] = []
 

+         project_dirnames = {}
 

+         data = {'project_dirnames': project_dirnames}
 

+ 

          for build in builds:
 

-             chroot_builddirs = cls.get_chroot_builddirs(build)
 

-             data["builds"].append(chroot_builddirs)
 

+             build_delete_data = cls.get_build_delete_data(build)
 

+ 

+             # inherit some params from the first build
 

+             for param in ['ownername', 'projectname']:
 

+                 new = build_delete_data[param]
 

+                 if param in data and data[param] != new:
 

+                     # this shouldn't happen
 

+                     raise exceptions.BadRequest("Can not delete builds "
 

+                                                 "from more projects")
 

+                 data[param] = new
 

+ 

+             dirname = build_delete_data['project_dirname']
 

+             if not dirname in project_dirnames:
 

+                 project_dirnames[dirname] = {}
 

+ 

+             project_dirname = project_dirnames[dirname]
 

+             for chroot, subdirs in build_delete_data['chroot_builddirs'].items():
 

+                 if chroot not in project_dirname:
 

+                     project_dirname[chroot] = subdirs
 

+                 else:
 

+                     project_dirname[chroot].extend(subdirs)
 

  

          action = models.Action(
 

              action_type=ActionTypeEnum("delete"),

file modified
+19 -1 
@@ -397,7 +397,8 @@
 

              user=self.u1,
 

              submitted_on=50,
 

              pkgs="http://example.com/copr-keygen-1.58-1.fc20.src.rpm",
 

-             pkg_version="1.58"
 

+             pkg_version="1.58",
 

+             result_dir='00000FEW',
 

          )
 

  

          self.db.session.add(self.b_few_chroots)
 
@@ -543,6 +544,23 @@
 

              source_type=0)
 

  

      @pytest.fixture
 

+     def f_pr_build(self, f_mock_chroots, f_builds, f_pr_dir):
 

+         self.b_pr = models.Build(
 

+             copr=self.c1, copr_dir=self.c4_dir, package=self.p1,
 

+             user=self.u1, submitted_on=50, srpm_url="http://somesrpm",
 

+             source_status=StatusEnum("succeeded"), result_dir='0000PR')
 

+ 

+         self.bc_pr = models.BuildChroot(
 

+             build=self.b_pr,
 

+             mock_chroot=self.mc2,
 

+             status=StatusEnum("succeeded"),
 

+             git_hash="deadbeef",
 

+             result_dir='0000PR-pr-package',
 

+         )
 

+ 

+         self.db.session.add_all([self.b_pr, self.bc_pr])
 

+ 

+     @pytest.fixture
 

      def f_batches(self):
 

          self.batch1 = models.Batch()
 

          self.batch2 = models.Batch()

file modified
+1 -1 
@@ -221,7 +221,7 @@
 

          assert len(ActionsLogic.get_many().all()) == 1
 

          action = ActionsLogic.get_many().one()
 

          delete_data = json.loads(action.data)
 

-         assert delete_data['chroot_builddirs'] == {'srpm-builds': 'bar', 'fedora-18-x86_64': 'bar'}
 

+         assert delete_data['chroot_builddirs'] == {'srpm-builds': ['bar'], 'fedora-18-x86_64': ['bar']}
 

  

          with pytest.raises(NoResultFound):
 

              BuildsLogic.get(self.b1.id).one()

file modified
+15 -7 
@@ -144,9 +144,9 @@
 

          self.b_few_chroots.build_chroots[1].status= StatusEnum("canceled")
 

          self.db.session.commit()
 

  

-         expected_chroot_builddirs = {'srpm-builds': self.b_few_chroots.result_dir}
 

+         expected_chroot_builddirs = {'srpm-builds': [self.b_few_chroots.result_dir]}
 

          for chroot in self.b_few_chroots.build_chroots:
 

-             expected_chroot_builddirs[chroot.name] = chroot.result_dir
 

+             expected_chroot_builddirs[chroot.name] = [chroot.result_dir]
 

  

          expected_dir = self.b_few_chroots.result_dir
 

          r = self.test_client.post(
 
@@ -214,8 +214,7 @@
 

      @TransactionDecorator("u1")
 

      def test_copr_delete_multiple_builds_sends_single_action(self, f_users,
 

                                                               f_coprs,
 

-                                                              f_mock_chroots,
 

-                                                              f_builds):
 

+                                                              f_pr_build):
 

          for bc in self.b2_bc:
 

              bc.status = StatusEnum("canceled")
 

          self.db.session.add_all(self.b2_bc)
 
@@ -224,10 +223,11 @@
 

  

          b_id1 = self.b1.id
 

          b_id2 = self.b2.id
 

+         b_id3 = self.b_pr.id
 

          url = "/coprs/{0}/{1}/delete_builds/".format(self.u1.name, self.c1.name)
 

  

          r = self.test_client.post(
 

-             url, data={"build_ids[]": [b_id1, b_id2]}, follow_redirects=True)
 

+             url, data={"build_ids[]": [b_id1, b_id2, b_id3]}, follow_redirects=True)
 

          assert r.status_code == 200
 

  

          b1 = (
 
@@ -249,8 +249,16 @@
 

          assert act.object_type == "builds"
 

          assert data.get('ownername') == "user1"
 

          assert data.get('projectname') == "foocopr"
 

-         assert data.get('builds') == 2 * [{'fedora-18-x86_64': 'bar',
 

-                                            'srpm-builds': 'bar'}]
 

+         assert data.get('project_dirnames') == {
 

+             'foocopr': {
 

+                 # they'd usually look like ID-PKGNAME, not 'bar'
 

+                 'fedora-18-x86_64': ['bar', 'bar'],
 

+                 'srpm-builds': ['bar', 'bar']},
 

+             'foocopr:PR': {
 

+                 'fedora-17-x86_64': ['0000PR-pr-package'],
 

+                 'srpm-builds': ['0000PR'],
 

+             }
 

+         }
 

  

      @TransactionDecorator("u1")
 

      def test_copr_delete_package_sends_single_action(self, f_users,

Previously we mixed-up actions that deleted buils with different
copr_dirs. While we are on it, let's minimize the action dictionary.

Fixes: #1180

rebased onto 9a3033b896a5714e7a4f1a6a04d900e5103be3b1
4 years ago

[copr-build]

rebased onto e5f8dca3b6d1124090aab4d6917f0b2fec4d9ad0
4 years ago

rebased onto eba632f45871f1999060e7bc7427d39e8c0dac98
4 years ago

I would rename inherit to parameter or something to make it more clear.

chroot_path could be created only once outside of the loop.

Otherwise +1.

rebased onto b9bf7b5
4 years ago

I updated the two minor things, so merging.

Commit b9bf7b5 fixes this pull-request

Pull-Request has been merged by praiskup
4 years ago

Pull-Request has been merged by praiskup
4 years ago
Changes Summary 6
+41 -22
file changed
backend/backend/actions.py
+5 -5
file changed
backend/tests/test_action.py
+31 -18
file changed
frontend/coprs_frontend/coprs/logic/actions_logic.py
+19 -1
file changed
frontend/coprs_frontend/tests/coprs_test_case.py
+1 -1
file changed
frontend/coprs_frontend/tests/test_logic/test_builds_logic.py
+15 -7
file changed
frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.