From 3def17077c85c9b55077208e9d4338edc654a21a Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Oct 09 2017 14:45:29 +0000
Subject: [PATCH 1/3] replace pycurl with requests


Related: https://pagure.io/koji/issue/600

---

diff --git a/cli/koji_cli/lib.py b/cli/koji_cli/lib.py
index 124780a..a1b6979 100644
--- a/cli/koji_cli/lib.py
+++ b/cli/koji_cli/lib.py
@@ -4,12 +4,12 @@ from __future__ import division
 import optparse
 import os
 import random
+import requests
 import six
 import socket
 import string
 import sys
 import time
-import pycurl
 from six.moves import range
 
 try:
@@ -481,33 +481,35 @@ def download_file(url, relpath, quiet=False, noprogress=False, size=None, num=No
             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 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))
diff --git a/docs/source/writing_koji_code.rst b/docs/source/writing_koji_code.rst
index 7f25f31..ee00c59 100644
--- a/docs/source/writing_koji_code.rst
+++ b/docs/source/writing_koji_code.rst
@@ -676,7 +676,6 @@ You will need to install the following packages to actually run the tests.
  * ``python-mock``
  * ``python-simplejson``
  * ``python-psycopg2``
- * ``python-pycurl``
  * ``python-requests``
  * ``python-qpid-proton``
 
diff --git a/koji.spec b/koji.spec
index 30f5266..23888d5 100644
--- a/koji.spec
+++ b/koji.spec
@@ -70,7 +70,6 @@ Requires: python2-rpm
 Requires: rpm-python
 %endif
 Requires: pyOpenSSL
-Requires: python-pycurl
 Requires: python-requests
 Requires: python-requests-kerberos
 Requires: python-dateutil
@@ -89,7 +88,6 @@ Requires: python%{python3_pkgversion}-rpm
 %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
diff --git a/tests/test_cli/test_download_file.py b/tests/test_cli/test_download_file.py
index e2b5783..2987e8e 100644
--- a/tests/test_cli/test_download_file.py
+++ b/tests/test_cli/test_download_file.py
@@ -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 @@ import unittest
 
 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,15 @@ class TestDownloadFile(unittest.TestCase):
         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()
+        self.requests_get = self.requests_get.return_value.__enter__
 
     def tearDown(self):
         mock.patch.stopall()
@@ -40,30 +45,55 @@ class TestDownloadFile(unittest.TestCase):
         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 +101,24 @@ class TestDownloadFile(unittest.TestCase):
         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 +131,14 @@ class TestDownloadProgress(unittest.TestCase):
         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'
diff --git a/vm/kojivmd b/vm/kojivmd
index 5491ee9..b678179 100755
--- a/vm/kojivmd
+++ b/vm/kojivmd
@@ -40,7 +40,7 @@ import SimpleXMLRPCServer
 import threading
 import base64
 import pwd
-import pycurl
+import requests
 import fnmatch
 from ConfigParser import ConfigParser
 from optparse import OptionParser
@@ -676,10 +676,16 @@ class VMExecTask(BaseTaskHandler):
             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 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')
 

From 7a505964581c1ca1d617c1f5996e89e23fa309cd Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Oct 09 2017 14:45:29 +0000
Subject: [PATCH 2/3] use contextlib wrapper for older python-requests


---

diff --git a/cli/koji b/cli/koji
index c88a0fb..19a9b25 100755
--- a/cli/koji
+++ b/cli/koji
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 # coding=utf-8
 
 # command line interface for the Koji build system
diff --git a/cli/koji_cli/lib.py b/cli/koji_cli/lib.py
index a1b6979..a464b5e 100644
--- a/cli/koji_cli/lib.py
+++ b/cli/koji_cli/lib.py
@@ -10,6 +10,7 @@ import socket
 import string
 import sys
 import time
+from contextlib import closing
 from six.moves import range
 
 try:
@@ -482,7 +483,8 @@ def download_file(url, relpath, quiet=False, noprogress=False, size=None, num=No
         else:
             print(_("Downloading: %s") % relpath)
 
-    with requests.get(url, stream=True) as response:
+
+    with closing(requests.get(url, stream=True)) as response:
         length = response.headers.get('content-length')
         f = open(relpath, 'wb')
         if length is None:
diff --git a/tests/test_cli/test_download_file.py b/tests/test_cli/test_download_file.py
index 2987e8e..158e730 100644
--- a/tests/test_cli/test_download_file.py
+++ b/tests/test_cli/test_download_file.py
@@ -32,7 +32,8 @@ class TestDownloadFile(unittest.TestCase):
         self.stdout = mock.patch('sys.stdout', new_callable=six.StringIO).start()
         self.stderr = mock.patch('sys.stderr', new_callable=six.StringIO).start()
         self.requests_get = mock.patch('requests.get', create=True, name='requests.get').start()
-        self.requests_get = self.requests_get.return_value.__enter__
+        # 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()
diff --git a/vm/kojivmd b/vm/kojivmd
index b678179..d2a2e90 100755
--- a/vm/kojivmd
+++ b/vm/kojivmd
@@ -43,6 +43,7 @@ import pwd
 import requests
 import fnmatch
 from ConfigParser import ConfigParser
+from contextlib import closing
 from optparse import OptionParser
 try:
     import krbV
@@ -676,7 +677,7 @@ class VMExecTask(BaseTaskHandler):
             else:
                 raise koji.BuildError('unsupported file type: %s' % type)
             koji.ensuredir(os.path.dirname(localpath))
-            with requests.get(remote_url, stream=True) as response:
+            with closing(requests.get(remote_url, stream=True)) as response:
                 f = open(localpath, 'wb')
                 length = response.headers.get('content-length')
                 if length is None:

From cdbd198663510b786018fe088ad34f2c3154070b Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Oct 16 2017 13:05:17 +0000
Subject: [PATCH 3/3] remove python3 typo


---

diff --git a/cli/koji b/cli/koji
index 19a9b25..c88a0fb 100755
--- a/cli/koji
+++ b/cli/koji
@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/python
 # coding=utf-8
 
 # command line interface for the Koji build system

