#2471 cli: download_file redownload if we've larger file
Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.
tkopecek/koji issue2465  into  master

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:

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

2 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
2 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.

ah, that makes sense, :thumbsup:

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

2 years ago

Commit ff055ad fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago