koji

Clone

#601 replace pycurl with requests
Merged 6 years ago by mikem. Opened 6 years ago by tkopecek.
tkopecek/koji issue600  into  master

file modified
+23 -19 
@@ -4,12 +4,13 @@ 
 

  import optparse
 

  import os
 

  import random
 

+ import requests
 

  import six
 

  import socket
 

  import string
 

  import sys
 

  import time
 

- import pycurl
 

+ from contextlib import closing
 

  from six.moves import range
 

  

  try:
 
@@ -481,33 +482,36 @@ 
 

              print(_("Downloading [%d/%d]: %s") % (num, size, relpath))
 

          else:
 

              print(_("Downloading: %s") % relpath)
 

-     c = pycurl.Curl()
 

-     c.setopt(c.URL, url)
 

-     # allow 301/302 redirect
 

-     c.setopt(pycurl.FOLLOWLOCATION, 1)
 

-     c.setopt(c.WRITEDATA, open(relpath, 'wb'))
 

-     if not (quiet or noprogress):
 

-         proc_func_param = getattr(c, 'XFERINFOFUNCTION', None)
 

-         if proc_func_param is None:
 

-             proc_func_param = getattr(c, 'PROGRESSFUNCTION', None)
 

-         if proc_func_param is not None:
 

-             c.setopt(c.NOPROGRESS, False)
 

-             c.setopt(proc_func_param, _download_progress)
 

+ 

+ 

+     with closing(requests.get(url, stream=True)) as response:
 

+         length = response.headers.get('content-length')
 

+         f = open(relpath, 'wb')
 

+         if length is None:
 

+             f.write(response.content)
 

+             length = len(response.content)
 

+             if not (quiet or noprogress):
 

+                 _download_progress(length, length)
 

          else:
 

-             c.close()
 

-             error(_('Error: XFERINFOFUNCTION and PROGRESSFUNCTION are not supported by pyCurl. Quit download progress'))
 

-     c.perform()
 

-     c.close()
 

+             l = 0
 

+             length = int(length)
 

+             for chunk in response.iter_content(chunk_size=65536):
 

+                 l += len(chunk)
 

+                 f.write(chunk)
 

+                 if not (quiet or noprogress):
 

+                     _download_progress(length, l)
 

+             f.close()
 

+ 

      if not (quiet or noprogress):
 

          print('')
 

  

  

- def _download_progress(download_t, download_d, upload_t, upload_d):
 

+ def _download_progress(download_t, download_d):
 

      if download_t == 0:
 

          percent_done = 0.0
 

      else:
 

          percent_done = float(download_d) / float(download_t)
 

-     percent_done_str = "%02d%%" % (percent_done * 100)
 

+     percent_done_str = "%3d%%" % (percent_done * 100)
 

      data_done = _format_size(download_d)
 

  

      sys.stdout.write("[% -36s] % 4s % 10s\r" % ('=' * (int(percent_done * 36)), percent_done_str, data_done))

file modified
-1 
@@ -676,7 +676,6 @@ 
 

   * ``python-mock``
 

   * ``python-simplejson``
 

   * ``python-psycopg2``
 

-  * ``python-pycurl``
 

   * ``python-requests``
 

   * ``python-qpid-proton``

file modified
-2 
@@ -70,7 +70,6 @@ 
 

  Requires: rpm-python
 

  %endif
 

  Requires: pyOpenSSL
 

- Requires: python-pycurl
 

  Requires: python-requests
 

  Requires: python-requests-kerberos
 

  Requires: python-dateutil
 
@@ -89,7 +88,6 @@ 
 

  %else
 

  Requires: rpm-python%{python3_pkgversion}
 

  %endif
 

- Requires: python%{python3_pkgversion}-pycurl
 

  Requires: python%{python3_pkgversion}-pyOpenSSL
 

  Requires: python%{python3_pkgversion}-requests
 

  Requires: python%{python3_pkgversion}-requests-kerberos

file modified
+61 -51 
@@ -1,6 +1,5 @@ 
 

  from __future__ import absolute_import
 

  import mock
 

- from mock import call
 

  import six
 

  import shutil
 

  import tempfile
 
@@ -8,6 +7,13 @@ 
 

  

  from koji_cli.lib import download_file, _download_progress
 

  

+ def mock_open():
 

+     """Return the right patch decorator for open"""
 

+     if six.PY2:
 

+         return mock.patch('__builtin__.open')
 

+     else:
 

+         return mock.patch('builtins.open')
 

+ 

  

  class TestDownloadFile(unittest.TestCase):
 

      # Show long diffs in error output...
 
@@ -18,16 +24,16 @@ 
 

          self.stdout.truncate()
 

          self.stderr.seek(0)
 

          self.stderr.truncate()
 

-         # self.curl.reset_mock()
 

-         self.curlClass.reset_mock()
 

+         self.requests_get.reset_mock()
 

  

      def setUp(self):
 

          self.tempdir = tempfile.mkdtemp()
 

          self.filename = self.tempdir + "/filename"
 

          self.stdout = mock.patch('sys.stdout', new_callable=six.StringIO).start()
 

          self.stderr = mock.patch('sys.stderr', new_callable=six.StringIO).start()
 

-         self.curlClass = mock.patch('pycurl.Curl', create=True).start()
 

-         self.curl = self.curlClass.return_value
 

+         self.requests_get = mock.patch('requests.get', create=True, name='requests.get').start()
 

+         # will work when contextlib.closing will be removed in future
 

+         #self.requests_get = self.requests_get.return_value.__enter__
 

  

      def tearDown(self):
 

          mock.patch.stopall()
 
@@ -40,30 +46,55 @@ 
 

          expected = 'Downloading: %s\n' % self.tempdir
 

          self.assertMultiLineEqual(actual, expected)
 

          self.assertEqual(cm.exception.args, (21, 'Is a directory'))
 

-         self.curlClass.assert_called_once()
 

-         self.assertEqual(self.curl.setopt.call_count, 2)
 

-         self.curl.perform.assert_not_called()
 

+         self.requests_get.assert_called_once()
 

+ 

+     @mock_open()
 

+     def test_handle_download_file(self, m_open):
 

+         self.reset_mock()
 

+         response = mock.MagicMock()
 

+         self.requests_get.return_value = response
 

+         response.headers.get.return_value = '5' # content-length
 

+         response.iter_content.return_value = ['abcde']
 

  

-     def test_handle_download_file(self):
 

          rv = download_file("http://url", self.filename)
 

+ 

          actual = self.stdout.getvalue()
 

-         expected = 'Downloading: %s\n\n' % self.filename
 

+         expected = 'Downloading: %s\n[====================================] 100%%     5.00 B\r\n' % self.filename
 

          self.assertMultiLineEqual(actual, expected)
 

-         self.curlClass.assert_called_once()
 

-         self.assertEqual(self.curl.setopt.call_count, 5)
 

-         self.curl.perform.assert_called_once()
 

-         self.curl.close.assert_called_once()
 

+ 

+         self.requests_get.assert_called_once()
 

+         m_open.assert_called_once()
 

+         response.headers.get.assert_called_once()
 

+         response.iter_content.assert_called_once()
 

          self.assertIsNone(rv)
 

  

+     @mock_open()
 

+     def test_handle_download_file_undefined_length(self, m_open):
 

+         self.reset_mock()
 

+         response = mock.MagicMock()
 

+         self.requests_get.return_value = response
 

+         response.headers.get.return_value = None # content-length
 

+         response.content = 'abcdef'
 

+ 

+         rv = download_file("http://url", self.filename)
 

+ 

+         actual = self.stdout.getvalue()
 

+         expected = 'Downloading: %s\n[====================================] 100%%     6.00 B\r\n' % self.filename
 

+         self.assertMultiLineEqual(actual, expected)
 

+ 

+         self.requests_get.assert_called_once()
 

+         m_open.assert_called_once()
 

+         response.headers.get.assert_called_once()
 

+         response.iter_content.assert_not_called()
 

+         self.assertIsNone(rv)
 

+ 

+ 

      def test_handle_download_file_with_size(self):
 

          rv = download_file("http://url", self.filename, size=10, num=8)
 

          actual = self.stdout.getvalue()
 

          expected = 'Downloading [8/10]: %s\n\n' % self.filename
 

          self.assertMultiLineEqual(actual, expected)
 

-         self.curlClass.assert_called_once()
 

-         self.assertEqual(self.curl.setopt.call_count, 5)
 

-         self.curl.perform.assert_called_once()
 

-         self.curl.close.assert_called_once()
 

+         self.requests_get.assert_called_once()
 

          self.assertIsNone(rv)
 

  

      def test_handle_download_file_quiet_noprogress(self):
 
@@ -71,45 +102,24 @@ 
 

          actual = self.stdout.getvalue()
 

          expected = ''
 

          self.assertMultiLineEqual(actual, expected)
 

-         self.assertEqual(self.curl.setopt.call_count, 3)
 

  

          self.reset_mock()
 

          download_file("http://url", self.filename, quiet=True, noprogress=True)
 

          actual = self.stdout.getvalue()
 

          expected = ''
 

          self.assertMultiLineEqual(actual, expected)
 

-         self.assertEqual(self.curl.setopt.call_count, 3)
 

  

          self.reset_mock()
 

          download_file("http://url", self.filename, quiet=False, noprogress=True)
 

          actual = self.stdout.getvalue()
 

          expected = 'Downloading: %s\n' % self.filename
 

          self.assertMultiLineEqual(actual, expected)
 

-         self.assertEqual(self.curl.setopt.call_count, 3)
 

- 

-     def test_handle_download_file_curl_version(self):
 

-         self.curl.XFERINFOFUNCTION = None
 

-         download_file("http://url", self.filename, quiet=False, noprogress=False)
 

-         actual = self.stdout.getvalue()
 

-         expected = 'Downloading: %s\n\n' % self.filename
 

-         self.assertMultiLineEqual(actual, expected)
 

-         self.assertEqual(self.curl.setopt.call_count, 5)
 

-         self.curl.setopt.assert_has_calls([call(self.curl.PROGRESSFUNCTION, _download_progress)])
 

- 

-         self.reset_mock()
 

-         self.curl.PROGRESSFUNCTION = None
 

-         with self.assertRaises(SystemExit) as cm:
 

-             download_file("http://url", self.filename, quiet=False, noprogress=False)
 

-         actual = self.stdout.getvalue()
 

-         expected = 'Downloading: %s\n' % self.filename
 

-         self.assertMultiLineEqual(actual, expected)
 

-         actual = self.stderr.getvalue()
 

-         expected = 'Error: XFERINFOFUNCTION and PROGRESSFUNCTION are not supported by pyCurl. Quit download progress\n'
 

-         self.assertMultiLineEqual(actual, expected)
 

-         self.assertEqual(self.curl.setopt.call_count, 3)
 

-         self.assertEqual(cm.exception.code, 1)
 

- 

  

+     '''
 

+     possible tests
 

+     - handling redirect headers
 

+     - http vs https
 

+     '''
 

  

  class TestDownloadProgress(unittest.TestCase):
 

      # Show long diffs in error output...
 
@@ -122,14 +132,14 @@ 
 

          mock.patch.stopall()
 

  

      def test_download_progress(self):
 

-         _download_progress(0, 0, None, None)
 

-         _download_progress(1024 * 92, 1024, None, None)
 

-         _download_progress(1024 * 1024 * 23, 1024 * 1024 * 11, None, None)
 

-         _download_progress(1024 * 1024 * 1024 * 35, 1024 * 1024 * 1024 * 30, None, None)
 

-         _download_progress(318921, 318921, None, None)
 

+         _download_progress(0, 0)
 

+         _download_progress(1024 * 92, 1024)
 

+         _download_progress(1024 * 1024 * 23, 1024 * 1024 * 11)
 

+         _download_progress(1024 * 1024 * 1024 * 35, 1024 * 1024 * 1024 * 30)
 

+         _download_progress(318921, 318921)
 

          actual = self.stdout.getvalue()
 

-         expected = '[                                    ]  00%     0.00 B\r' + \
 

-                    '[                                    ]  01%   1.00 KiB\r' + \
 

+         expected = '[                                    ]   0%     0.00 B\r' + \
 

+                    '[                                    ]   1%   1.00 KiB\r' + \
 

                     '[=================                   ]  47%  11.00 MiB\r' + \
 

                     '[==============================      ]  85%  30.00 GiB\r' + \
 

                     '[====================================] 100% 311.45 KiB\r'

file modified
+12 -5 
@@ -40,9 +40,10 @@ 
 

  import threading
 

  import base64
 

  import pwd
 

- import pycurl
 

+ import requests
 

  import fnmatch
 

  from ConfigParser import ConfigParser
 

+ from contextlib import closing
 

  from optparse import OptionParser
 

  try:
 

      import krbV
 
@@ -676,10 +677,16 @@ 
 

              else:
 

                  raise koji.BuildError('unsupported file type: %s' % type)
 

              koji.ensuredir(os.path.dirname(localpath))
 

-             c = pycurl.Curl()
 

-             c.setopt(c.URL, remote_url)
 

-             c.setopt(c.WRITEDATA, open(localpath, 'wb'))
 

-             c.perform()
 

+             with closing(requests.get(remote_url, stream=True)) as response:
 

+                 f = open(localpath, 'wb')
 

+                 length = response.headers.get('content-length')
 

+                 if length is None:
 

+                     f.write(response.content)
 

+                 else:
 

+                     for chunk in response.iter_content(chunk_size=65536):
 

+                         f.write(chunk)
 

+                 f.close()
 

+ 

  

          return file(localpath, 'r')

1 new commit added

  • use contextlib wrapper for older python-requests
6 years ago

2 new commits added

  • use contextlib wrapper for older python-requests
  • replace pycurl with requests
6 years ago

This seems like a spurious change?

rebased onto 3def170
6 years ago

Rebased and removed python3 typo.
Edited 6 years ago by tkopecek

Why change the #! line in the checkout? Won't that carry over to the build even when building without python3 support?

1 new commit added

  • remove python3 typo
6 years ago

It shouldn't be there. I somehow managed to push it again...

:thumbsup:

Commit 84d0f8e fixes this pull-request

Pull-Request has been merged by mikem@redhat.com
6 years ago
Changes Summary 5
+23 -19
file changed
cli/koji_cli/lib.py
+0 -1
file changed
docs/source/writing_koji_code.rst
+0 -2
file changed
koji.spec
+61 -51
file changed
tests/test_cli/test_download_file.py
+12 -5
file changed
vm/kojivmd
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.