koji

Clone

#1542 Replace urllib.request with requests library
Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.
tkopecek/koji issue1530  into  master

file modified
+3 -3 
@@ -1660,10 +1660,10 @@ 
 

      on options"""
 

      if topurl:
 

          url = "%s/%s" % (topurl, relpath)
 

-         src = six.moves.urllib.request.urlopen(url)
 

          fo = tempfile.TemporaryFile(dir=tempdir)
 

-         shutil.copyfileobj(src, fo)
 

-         src.close()
 

+         with requests.get(url) as resp:
 

+             for chunk in resp.iter_content(chunk_size=8192):
 

+                 fo.write(chunk)
 

          fo.seek(0)
 

      elif topdir:
 

          fn = "%s/%s" % (topdir, relpath)

file modified
+7 -7 
@@ -29,8 +29,8 @@ 
 

  import signal
 

  import time
 

  

+ import requests
 

  import six.moves.xmlrpc_client
 

- import six.moves.urllib.request
 

  from six.moves import range
 

  

  import koji
 
@@ -479,12 +479,12 @@ 
 

                  return fn
 

              self.logger.debug("Downloading %s", relpath)
 

              url = "%s/%s" % (self.options.topurl, relpath)
 

-             fsrc = six.moves.urllib.request.urlopen(url)
 

-             if not os.path.exists(os.path.dirname(fn)):
 

-                 os.makedirs(os.path.dirname(fn))
 

-             with open(fn, 'wb') as fdst:
 

-                 shutil.copyfileobj(fsrc, fdst)
 

-             fsrc.close()
 

+             with requests.get(url) as resp:
 

+                 if not os.path.exists(os.path.dirname(fn)):
 

+                     os.makedirs(os.path.dirname(fn))
 

+                 with open(fn, 'wb') as fdst:
 

+                     for chunk in resp.iter_content(chunk_size=8192):
 

+                         fdst.write(chunk)
 

          else:
 

              fn = "%s/%s" % (self.options.topdir, relpath)
 

          return fn

file modified
+2 -1 
@@ -62,6 +62,7 @@ 
 

  

      def setUp(self):
 

          self.ksession = koji.ClientSession('http://koji.example.com/kojihub')
 

+         self.ksession.logout = mock.MagicMock()
 

          self.do_fake_login()
 

          # mocks
 

          self.ksession._callMethod = mock.MagicMock()
 
@@ -89,7 +90,6 @@ 
 

              self.ksession.fastUpload('nonexistent_file', 'target')
 

  

      def test_fastUpload_nofile(self):
 

- 

          # fail with nonexistent file (IOError)
 

          self.file_mock.side_effect = IOError('mocked exception')
 

          with self.assertRaises(IOError):
 
@@ -180,6 +180,7 @@ 
 

          self.ksession = koji.ClientSession('http://koji.example.com/kojihub')
 

          # mocks
 

          self.ksession._sendCall = mock.MagicMock()
 

+         self.ksession.logout = mock.MagicMock()
 

  

      def tearDown(self):
 

          del self.ksession

file modified
+7 -3 
@@ -11,6 +11,7 @@ 
 

  from os import path, makedirs
 

  from tempfile import gettempdir
 

  from mock import patch, MagicMock, Mock, call
 

+ import requests_mock
 

  

  import koji
 

  from koji.tasks import BaseTaskHandler, FakeTask, ForkTask, SleepTask, \
 
@@ -452,8 +453,8 @@ 
 

          obj = TestTask(123, 'some_method', ['random_arg'], None, options, temp_path)
 

          self.assertEquals(obj.localPath('test.txt'), dummy_file)
 

  

-     @patch('six.moves.urllib.request.urlopen', return_value=six.BytesIO(six.b('Important things\nSome more important things\n')))
 

-     def test_BaseTaskHandler_localPath_no_file(self, mock_urlopen):
 

+     @requests_mock.Mocker()
 

+     def test_BaseTaskHandler_localPath_no_file(self, m_requests):
 

          """
 

          """
 

          temp_path = get_tmp_dir_path('TestTask')
 
@@ -466,10 +467,13 @@ 
 

  

          options = Mock()
 

          options.topurl = 'https://www.domain.local'
 

+         url = options.topurl + '/test.txt'
 

+         m_requests.register_uri('GET', url, text='Important things\nSome more important things\n')
 

          obj = TestTask(123, 'some_method', ['random_arg'], None, options, temp_path)
 

  

          self.assertEquals(obj.localPath('test.txt'), target_file_path)
 

-         mock_urlopen.assert_called_once_with('https://www.domain.local/test.txt')
 

+         self.assertEquals(m_requests.call_count, 1)
 

+         self.assertEquals(m_requests.request_history[0].url, url)
 

  

      def test_BaseTaskHandler_localPath_no_topurl(self):
 

          """ Tests that the localPath function returns a path when options.topurl is not defined.

file modified
+40 -37 
@@ -14,6 +14,7 @@ 
 

  except ImportError:
 

      import unittest
 

  

+ import requests_mock
 

  from mock import call, patch
 

  from datetime import datetime
 

  import koji
 
@@ -101,62 +102,64 @@ 
 

          move.assert_not_called()
 

  

      @mock_open()
 

-     @mock.patch('six.moves.urllib.request.urlopen')
 

      @mock.patch('tempfile.TemporaryFile')
 

-     @mock.patch('shutil.copyfileobj')
 

-     def test_openRemoteFile(self, m_copyfileobj, m_TemporaryFile,
 

-                             m_urlopen, m_open):
 

+     def test_openRemoteFile(self, m_TemporaryFile, m_open):
 

          """Test openRemoteFile function"""
 

  

-         mocks = [m_open, m_copyfileobj, m_TemporaryFile, m_urlopen]
 

+         mocks = [m_open, m_TemporaryFile ]
 

  

          topurl = 'http://example.com/koji'
 

          path = 'relative/file/path'
 

          url = 'http://example.com/koji/relative/file/path'
 

- 

-         # using topurl, no tempfile
 

-         fo = koji.openRemoteFile(path, topurl)
 

-         m_urlopen.assert_called_once_with(url)
 

-         m_urlopen.return_value.close.assert_called_once()
 

-         m_TemporaryFile.assert_called_once_with(dir=None)
 

-         m_copyfileobj.assert_called_once()
 

-         m_open.assert_not_called()
 

-         assert fo is m_TemporaryFile.return_value
 

+         with requests_mock.Mocker() as m_requests:
 

+             m_requests.register_uri('GET', url, text='random content')
 

+             # using topurl, no tempfile
 

+             fo = koji.openRemoteFile(path, topurl)
 

+             self.assertEqual(m_requests.call_count, 1)
 

+             self.assertEqual(m_requests.request_history[0].url, url)
 

+             m_TemporaryFile.assert_called_once_with(dir=None)
 

+             m_open.assert_not_called()
 

+             assert fo is m_TemporaryFile.return_value
 

  

          for m in mocks:
 

              m.reset_mock()
 

  

-         # using topurl + tempfile
 

-         tempdir = '/tmp/koji/1234'
 

-         fo = koji.openRemoteFile(path, topurl, tempdir=tempdir)
 

-         m_urlopen.assert_called_once_with(url)
 

-         m_urlopen.return_value.close.assert_called_once()
 

-         m_TemporaryFile.assert_called_once_with(dir=tempdir)
 

-         m_copyfileobj.assert_called_once()
 

-         m_open.assert_not_called()
 

-         assert fo is m_TemporaryFile.return_value
 

+         with requests_mock.Mocker() as m_requests:
 

+             m_requests.register_uri('GET', url, text='random content')
 

+ 

+             # using topurl + tempfile
 

+             tempdir = '/tmp/koji/1234'
 

+             fo = koji.openRemoteFile(path, topurl, tempdir=tempdir)
 

+             self.assertEqual(m_requests.call_count, 1)
 

+             self.assertEqual(m_requests.request_history[0].url, url)
 

+             m_TemporaryFile.assert_called_once_with(dir=tempdir)
 

+             m_open.assert_not_called()
 

+             assert fo is m_TemporaryFile.return_value
 

  

          for m in mocks:
 

              m.reset_mock()
 

  

-         # using topdir
 

-         topdir = '/mnt/mykojidir'
 

-         filename = '/mnt/mykojidir/relative/file/path'
 

-         fo = koji.openRemoteFile(path, topdir=topdir)
 

-         m_urlopen.assert_not_called()
 

-         m_TemporaryFile.assert_not_called()
 

-         m_copyfileobj.assert_not_called()
 

-         m_open.assert_called_once_with(filename)
 

-         assert fo is m_open.return_value
 

+         with requests_mock.Mocker() as m_requests:
 

+             m_requests.register_uri('GET', url, text='random content')
 

+             # using topdir
 

+             topdir = '/mnt/mykojidir'
 

+             filename = '/mnt/mykojidir/relative/file/path'
 

+             fo = koji.openRemoteFile(path, topdir=topdir)
 

+             self.assertEqual(m_requests.call_count, 0)
 

+             m_TemporaryFile.assert_not_called()
 

+             m_open.assert_called_once_with(filename)
 

+             assert fo is m_open.return_value
 

  

          for m in mocks:
 

              m.reset_mock()
 

  

-         # using neither
 

-         with self.assertRaises(koji.GenericError):
 

-             koji.openRemoteFile(path)
 

-         for m in mocks:
 

-             m.assert_not_called()
 

+         with requests_mock.Mocker() as m_requests:
 

+             m_requests.register_uri('GET', url, text='random content')
 

+             # using neither
 

+             with self.assertRaises(koji.GenericError):
 

+                 koji.openRemoteFile(path)
 

+             for m in mocks:
 

+                 m.assert_not_called()
 

  

      def test_joinpath_bad(self):
 

          bad_joins = [

Update of #294
Depends on #1185 (request-mock inclusion)

:thumbsup:

rebased onto 5b9ef98
4 years ago

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

:thumbsup:

Do we already include requests as a dependency in setup.py and the rpm spec?

Yes, it is in both.

:thumbsup:

Commit f2774d5 fixes this pull-request

Pull-Request has been merged by tkopecek
4 years ago

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

context manager:

with request.get(url) as resp:
    pass

is not supported on EL7
- RHEL7: 2.6.0
- EPEL7: 2.3.0
Changes Summary 5
+3 -3
file changed
koji/__init__.py
+7 -7
file changed
koji/tasks.py
+2 -1
file changed
tests/test_lib/test_client_session.py
+7 -3
file changed
tests/test_lib/test_tasks.py
+40 -37
file changed
tests/test_lib/test_utils.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.