PR#723: [frontend] add command to delete orphaned builds and packages - copr/copr

copr / copr

#723 [frontend] add command to delete orphaned builds and packages

Merged 4 years ago by msuchy. Opened 5 years ago by dturecek.

copr/ dturecek/copr orphaned-builds into master

[frontend] add command to delete orphaned builds and packages

Dominik Turecek • 4 years ago

5e2c722

frontend/coprs_frontend/commands/delete_orphans.py

file added

+13

		`@@ -0,0 +1,13 @@`
		`+ from flask_script import Command`
		`+ from coprs.logic.builds_logic import BuildsLogic`
		`+ from coprs.logic.packages_logic import PackagesLogic`
		`+`
		`+`
		`+ class DeleteOrphansCommand(Command):`
		`+ """`
		`+ Deletes builds and packages associated to deleted coprs.`
		`+ """`
		`+`
		`+ def run(self):`
		`+ BuildsLogic.delete_orphaned_builds()`
		`+ PackagesLogic.delete_orphaned_packages()`
praiskup commented 5 years ago Deleting package might trigger build removal; I'd prefer to first call delete_orphaned packages.
praiskup commented 5 years ago Taking back, deleting builds first means that we'll have more smooth removal (removing of one package with tousands of builds might be much prone to errors).

frontend/coprs_frontend/coprs/logic/actions_logic.py

file modified

+5 -1

		`@@ -112,10 +112,14 @@`
		`data_dict = {`
		`"ownername": build.copr.owner_name,`
		`"projectname": build.copr_name,`
		`- "project_dirname": build.copr_dirname,`
		`"chroot_builddirs": chroot_builddirs,`
		`}`

		`+ if build.copr_dir:`
		`+ data_dict["project_dirname"] = build.copr_dirname`
		`+ else:`
		`+ data_dict["project_dirname"] = build.copr_name`
		`+`
		`action = models.Action(`
		`action_type=ActionTypeEnum("delete"),`
		`object_type="build",`

frontend/coprs_frontend/coprs/logic/builds_logic.py

file modified

+18

		`@@ -1141,6 +1141,24 @@`
		`# postpone this one to next day run`
		`log.error("Build(id={}) delete failed, unfinished action.".format(build.id))`

		`+ @classmethod`
		`+ def delete_orphaned_builds(cls):`
		`+ builds_to_delete = models.Build.query\`
		`+ .join(models.Copr, models.Build.copr_id == models.Copr.id)\`
		`+ .filter(models.Copr.deleted == True)`
		`+`
		`+ counter = 0`
		`+ for build in builds_to_delete:`
		`+ cls.delete_build(build.copr.user, build)`
		`+ counter += 1`
		`+ if counter >= 100:`
		`+ db.session.commit()`
		`+ counter = 0`
		`+`
		`+ if counter > 0:`
		`+ db.session.commit()`
		`+`
		`+`
		`class BuildChrootsLogic(object):`
		`@classmethod`
		`def get_by_build_id_and_name(cls, build_id, name):`

frontend/coprs_frontend/coprs/logic/packages_logic.py

file modified

+17

		`@@ -295,3 +295,20 @@`
		`new_builds.append(new_build)`

		`return new_builds`
		`+`
		`+ @classmethod`
		`+ def delete_orphaned_packages(cls):`
		`+ pkgs_to_delete = models.Package.query\`
		`+ .join(models.Copr, models.Package.copr_id == models.Copr.id)\`
		`+ .filter(models.Copr.deleted == True)`
		`+`
		`+ counter = 0`
		`+ for pkg in pkgs_to_delete:`
		`+ cls.delete_package(pkg.copr.user, pkg)`
		`+ counter += 1`
		`+ if counter >= 100:`
		`+ db.session.commit()`
		`+ counter = 0`
		`+`
		`+ if counter > 0:`
		`+ db.session.commit()`

frontend/coprs_frontend/manage.py

file modified

		`@@ -42,6 +42,7 @@`
		`"delete_outdated_chroots": "DeleteOutdatedChrootsCommand",`
		`"clean_expired_projects": "CleanExpiredProjectsCommand",`
		`"clean_old_builds": "DeleteOldBuilds",`
		`+ "delete_orphans": "DeleteOrphansCommand",`
		`}`

		`if os.getuid() == 0:`

no initial comment

rebased onto 9066cb93cc57e86cf64ce6bab95c76bfd344462b

5 years ago

dturecek commented 5 years ago

Based on a discussion with @msuchy, I've added counters to commit after every 1000 packages/builds deleted.

praiskup commented on line 14 of frontend/coprs_frontend/commands/delete_orphans.py 5 years ago

Deleting package might trigger build removal; I'd prefer to first call delete_orphaned packages.

praiskup commented on line 16 of frontend/coprs_frontend/coprs/logic/packages_logic.py 5 years ago

Here I'd use even smaller number.

praiskup commented 5 years ago

nice PR, thank you

praiskup commented on line 20 of frontend/coprs_frontend/coprs/logic/packages_logic.py 5 years ago

This might by accident end with double commit and traceback. You probably want "if counter > 1".

praiskup commented on line 14 of frontend/coprs_frontend/commands/delete_orphans.py 5 years ago

Taking back, deleting builds first means that we'll have more smooth removal (removing of one package with tousands of builds might be much prone to errors).

praiskup commented on line 20 of frontend/coprs_frontend/coprs/logic/packages_logic.py 5 years ago

I meant counter > 0.

rebased onto 57bd86c892655c431b850202dba8e4b03096d265

5 years ago

dturecek commented 5 years ago

I've lowered the number of deleted builds/packages per commit to 100 and fixed the possible double commit at the end.

praiskup commented 5 years ago

Ah, now I realized the problem ... some of the packages don't belong to any coprdir:

praiskup=# select count(*) from package where copr_dir_id is null;
 count 
-------
 85372
(1 row)

praiskup=# select count(*) from build where copr_dir_id is null;
 count 
-------
  5833
(1 row)

I tried to execute locally, and I received this:

Traceback (most recent call last):
  File "./manage.py", line 62, in <module>
    manager.run()
  File "/usr/lib/python3.7/site-packages/flask_script/__init__.py", line 417, in run
    result = self.handle(argv[0], argv[1:])
  File "/usr/lib/python3.7/site-packages/flask_script/__init__.py", line 386, in handle
    res = handle(*args, **config)
  File "/usr/lib/python3.7/site-packages/flask_script/commands.py", line 216, in __call__
    return self.run(*args, **kwargs)
  File "/copr/coprs_frontend/commands/delete_orphans.py", line 12, in run
    BuildsLogic.delete_orphaned_builds()
  File "/copr/coprs_frontend/coprs/logic/builds_logic.py", line 1152, in delete_orphaned_builds
    cls.delete_build(build.copr.user, build)
  File "/copr/coprs_frontend/coprs/logic/builds_logic.py", line 1054, in delete_build
    ActionsLogic.send_delete_build(build)
  File "/copr/coprs_frontend/coprs/logic/actions_logic.py", line 115, in send_delete_build
    "project_dirname": build.copr_dirname,
  File "/copr/coprs_frontend/coprs/models.py", line 849, in copr_dirname
    return self.copr_dir.name
AttributeError: 'NoneType' object has no attribute 'name'

praiskup commented 5 years ago

Btw., can you please reference issue id in commit message?