#601 replace pycurl with requests
Merged 4 years ago by mikem. Opened 4 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))

@@ -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

@@ -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
4 years ago

2 new commits added

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

This seems like a spurious change?

rebased onto 3def170

4 years ago

Rebased and removed python3 typo.

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
4 years ago

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

Commit 84d0f8e fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

4 years ago