#3311 New feature: command download-scratch-build is added
Closed by geoargyr. Opened by geoargyr.
geoargyr/koji downlscratch  into  master

Download 3311.patch
  • anon_handle_download_scratch_build() method implements the command
  • test_download_scratch_build.py holds all testing for the new method
  • Updates for supporting the new command

This Pull Request is a possible solution to the following issue: https://pagure.io/koji/issue/3182

--topurl is already toplevel option (see koji --help, so it is a duplicate here).

--topurl is already toplevel option (see koji --help, so it is a duplicate here).

The anon_handle_download_scratch_build() function is implemented like functions anon_handle_download_build() and anon_handle_download_task().
They also have a --topurl option. Why is it considered only now duplicate?

hmm, you're right. We've to cleanup systematically in the future :-( So, ignore that comment.
Anyway, it is now working only for images. If we want to introduce new command it should be also able to download (at least) rpm scratch builds.

$ koji -p fedora download-scratch-build 85288881
No such scratch image build

@mikem I spoked with @tkopecek and current idea it to extend this PR for all builds where is --scratch option. What do you think? Are you agree with this idea?

As-is, this code doesn't really download from the scratch location. It downloads the task output paths, as returned by session.listTaskOutput(task_id, all_volumes=True). This is basically the same thing that download-task does.

Having a more convenient way to download scratch builds seems reasonable, but as Tomas rightly points out, it should handle all kinds of scratch builds.

Also, I'm not sure this needs its own command. Supporting a --scratch option in download-build would probably make more sense.

@mikem sure, I make sense to use download-build with --scratch option. So ok, I'll create new PR with updated download-build and use --scratch option for all kinds of scratch builds. Thanks Mike.

Actually, the best way to address the original issue, #3182, would be to fix the download-task command

Maybe we can get a separate issue filed for supporting download-build --scratch, as this is quite a different from #3182

Hello all! As I worked on this issue/feature for some time, I'd like to say my opinion hoping that it will be helpful for you.
When I started working on this, I checked the other commands and realized that download-task downloads non-scratch rpms and scratch rpms, while the download-build downloads non-scratch images and non-scratch rpms. So, for some reason the scratch images are left outside, while the non-scratch rpms can be served in two ways!
As a user, it would make much more sense if I had a --scratch option in download-build, but as a developer I complied with the initial proposal (as stated by tkopecek here: https://pagure.io/koji/issue/3182) and created a new download-scratch-build. Nevertheless, -thinking again as a developer- it will be an overhead to implement a --scratch option in download-build, as its functionality is based on the buildID and the scratch image builds do not have a buildID (as far as I know).
Also, as a developer, based on the fact that scratch image builds have no buildID, I based the functionality of anon_handle_download_scratch_build() on anon_handle_download_task(), so extending the download-task command in a way that downloads scratch image builds seems more reasonable from the developer point of view.

I tested the functionality of PR #3343 and it satisfies the https://pagure.io/koji/issue/3182
Based on that I am going to withdraw my PR #3311

Pull-Request has been closed by geoargyr

Metadata