PR#2471: cli: download_file redownload if we've larger file - koji

koji

#2471 cli: download_file redownload if we've larger file

Merged 3 years ago by tkopecek. Opened 3 years ago by tkopecek.

tkopecek/koji issue2465 into master

don't check size for signed rpms

Tomas Kopecek • 3 years ago

c5a233c

cli: download_file redownload if we've larger file

Tomas Kopecek • 3 years ago

0db4c09

cli/koji_cli/lib.py

file modified

+4 -2

		`@@ -527,7 +527,7 @@`

		`# closing needs to be used for requests < 2.18.0`
		`with closing(requests.get(url, headers=headers, stream=True)) as response:`
		`- if response.status_code == 200: # full content provided?`
		`+ if response.status_code in (200, 416): # full content provided or reaching behing EOF`
		`# rewrite in such case`
		`f.close()`
		`f = open(relpath, 'wb')`
		`@@ -551,12 +551,14 @@`
		`pi = koji.PathInfo(topdir=topurl)`
		`if sigkey:`
		`fname = pi.signed(rpm, sigkey)`
		`+ filesize = None`
		`else:`
		`fname = pi.rpm(rpm)`
		`+ filesize = rpm['size']`
		`url = os.path.join(pi.build(build), fname)`
		`path = os.path.basename(fname)`

		`- download_file(url, path, quiet=quiet, noprogress=noprogress, filesize=rpm['size'])`
		`+ download_file(url, path, quiet=quiet, noprogress=noprogress, filesize=filesize)`

		`# size - we have stored size only for unsigned copies`
		`if not sigkey:`

tkopecek commented 3 years ago

Fixes: https://pagure.io/koji/issue/2465

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

3 years ago

julian8628 commented 3 years ago

I think the root cause is

download_file(url, path, quiet=quiet, noprogress=noprogress, filesize=rpm['size'])

in download_rpm

If downloaded rpm is signed after being imported, filesize=rpm['size'] is the actual size of the signed rpm. It is the size of the imported(by import_rpm()) one.

Maybe we could just pass filesize=None when downloading a signed RPM?

1 new commit added

don't check size for signed rpms

3 years ago

tkopecek commented 3 years ago

@julian8628 hmm, it is another part of the problem - but yes, you're right. There are two issues - signed rpm should be fixed by your hint. But - if I've downloaded signed rpm (or something else with same name), which is longer than what unsigned rpm expects, download_file will send request header for range beyond EOF of real file. In such case we should redownload it completely, because it is obviously different file. So, there is where my code should catch it.

julian8628 commented 3 years ago

ah, that makes sense, :thumbsup: