koji

#658 consolidate safe_rmtree, rmtree and shutil.rmtree
Merged by mikem. Opened by tkopecek.
tkopecek/koji issue648  into  master

Download 658.patch

shutil.rmtree should be avoided in almost all cases
safe_rmtree has its usage in tasks module, but innards are replaced with
koji.util.rmtree, so we don't have two implementations of same task

Related: https://pagure.io/koji/issue/648

need to adjust unit test for recycle_build
safe_rmtree must return 0 on success (checked in updateBuildroots)
stale comment still refers to find command
might was well fix that bare except

changes here:
https://github.com/mikem23/koji-playground/commits/pagure/pr/658

ah, and I need to update the unit test again for that

Hmm, doesn't look like you are setting up the /mnt/folder that these new tests refer to.

The TasksTestCase class is kind of cluttered and I'm not sure these new tests benefit from being in there. Maybe it would be better to have a separate class with its own setUp/tearDown?

rebased onto 5acf3f9f8fdd3346a34c87eb4635bb94ad893422

Directory path is not touched, as all system calls are mocked. I think it is not necessary to set up real files, as all os.* calls are mocked.

Created separate class for safe_rmtree tests.

Directory path is not touched, as all system calls are mocked. I think it is not necessary to set up real files, as all os.* calls are mocked.

ah , of course

For completeness, we should also change the embedded rmtree code in delete/reset_build. Change added here:

https://github.com/mikem23/koji-playground/commits/pagure/pr/658

1 new commit added

  • also use rmtree in delete_build/reset_build

Updated with your commit.

rebased onto 09182866a5194181c127fd359ff8ba4301907f02

Commit 423eac9e fixes this pull-request

Pull-Request has been merged by mikem@redhat.com
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.