copr / copr

Clone

#2102 cli: don't raise immediately if cli can't connect to frontend
Merged 2 years ago by frostyx. Opened 2 years ago by schlupov.
copr/ schlupov/copr fix_watch_build  into  main

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

  

          if not self.config.get("token"):
 

              self.config["gssapi"] = True
 

+         self.config["connection_attempts"] = 3
 

          self.client = Client(self.config)
 

  

      def requires_api_auth(func):

file modified
+5 -5 
@@ -7,7 +7,7 @@ 
 

  

  @mock.patch.object(Request, "send")
 

  class TestBuildProxy(object):
 

-     config = {"copr_url": "http://copr"}
 

+     config = {"copr_url": "http://copr", "login": "test", "token": "test"}
 

  

      def test_get(self, send):
 

          response = mock.Mock(spec=Response)
 
@@ -20,14 +20,14 @@ 
 

          assert build.foo == "bar"
 

  

  

- @mock.patch("copr.v3.proxies.build.Request")
 

- def test_build_distgit(request):
 

+ @mock.patch('copr.v3.proxies.Request.send')
 

+ def test_build_distgit(send):
 

      mock_client = Client.create_from_config_file(config_location)
 

      mock_client.build_proxy.create_from_distgit(
 

          "praiskup", "ping", "mock", committish="master",
 

      )
 

-     assert len(request.call_args_list) == 1
 

-     call = request.call_args_list[0]
 

+     assert len(send.call_args_list) == 1
 

+     call = send.call_args_list[0]
 

      args = call[1]
 

      assert args['method'] == 'POST'
 

      assert args['endpoint'] == '/build/create/distgit'

file modified
+3 -2 
@@ -39,7 +39,8 @@ 
 

  

      @mock.patch("copr.v3.proxies.build.BuildProxy.get")
 

      def test_wait_custom_list(self, mock_get):
 

-         builds = List([Munch(id=1, state="succeeded"), Munch(id=2, state="failed")], proxy=BuildProxy({}))
 

+         builds = List([Munch(id=1, state="succeeded"), Munch(id=2, state="failed")],
 

+                       proxy=BuildProxy({"copr_url": "http://copr", "login": "test", "token": "test"}))
 

          mock_get.side_effect = lambda id: builds[id-1]
 

          assert wait(builds)
 

  
@@ -65,4 +66,4 @@ 
 

  

  

  class MunchMock(Munch):
 

-     __proxy__ = BuildProxy({})
 

+     __proxy__ = BuildProxy({"copr_url": "http://copr", "login": "test", "token": "test"})

file modified
+2 -2 
@@ -9,7 +9,7 @@ 
 

  

  

  @pytest.mark.parametrize('method_name', ['add', 'edit'])
 

- @mock.patch("copr.v3.proxies.package.Request")
 

+ @mock.patch("copr.v3.proxies.Request.send")
 

  def test_package_distgit(request, method_name):
 

      mock_client = Client.create_from_config_file(config_location)
 

      method = getattr(mock_client.package_proxy, method_name)
 
@@ -17,7 +17,7 @@ 
 

             {"committish": "master", "distgit": "fedora"})
 

      assert len(request.call_args_list) == 1
 

      call = request.call_args_list[0]
 

-     endpoint = call[0][0]
 

+     endpoint = call[1]["endpoint"]
 

      args = call[1]
 

      assert args['method'] == 'POST'
 

      base_url = "/package/{0}".format(method_name)

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

- import pytest
 

  from requests import Response
 

  from copr.test import mock
 

- from copr.v3.requests import Request, handle_errors, CoprRequestException, munchify
 

+ from copr.v3.requests import Request, munchify
 

  

  

  class TestResponse(object):
 
@@ -20,17 +19,19 @@ 
 

  

  class TestRequest(object):
 

      def test_endpoint_url(self):
 

-         r1 = Request(endpoint="foo", api_base_url="http://copr/api_3")
 

+         r1 = Request(api_base_url="http://copr/api_3")
 

+         r1.endpoint = "foo"
 

          assert r1.endpoint_url == "http://copr/api_3/foo"
 

  

          # Leading and/or trailing slash should not be a problem
 

-         r2 = Request(endpoint="/foo/bar", api_base_url="http://copr/api_3/")
 

+         r2 = Request(api_base_url="http://copr/api_3/")
 

+         r2.endpoint = "/foo/bar"
 

          assert r2.endpoint_url == "http://copr/api_3/foo/bar"
 

  

      @mock.patch('requests.Session.request')
 

      def test_send(self, request):
 

-         req1 = Request(endpoint="foo", api_base_url="http://copr/api_3")
 

-         resp1 = req1.send()
 

+         req1 = Request(api_base_url="http://copr/api_3")
 

+         resp1 = req1.send(endpoint="foo")
 

  

          request.assert_called_once()
 

          args, kwargs = request.call_args

file modified
+4 -4 
@@ -21,6 +21,7 @@ 
 

      def __init__(self, config):
 

          self.config = config
 

          self._auth_token_cached = None
 

+         self.request = Request(api_base_url=self.api_base_url, connection_attempts=config.get("connection_attempts", 1))
 

  

      @classmethod
 

      def create_from_config_file(cls, path=None):
 
@@ -71,8 +72,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = ""
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)
 

  

      def auth_check(self):
 
@@ -82,6 +82,6 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/auth-check"
 

-         request = Request(endpoint, api_base_url=self.api_base_url, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)

file modified
+22 -26 
@@ -2,7 +2,7 @@ 
 

  

  import os
 

  from . import BaseProxy
 

- from ..requests import Request, FileRequest, munchify, POST
 

+ from ..requests import FileRequest, munchify, POST
 

  from ..exceptions import CoprValidationException
 

  from ..helpers import for_all_methods, bind_proxy
 

  
@@ -17,8 +17,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)
 

  

      def get_source_chroot(self, build_id):
 
@@ -29,8 +28,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/source-chroot/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)
 

  

      def get_source_build_config(self, build_id):
 
@@ -41,8 +39,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/source-build-config/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)
 

  

      def get_built_packages(self, build_id):
 
@@ -53,8 +50,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/built-packages/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)
 

  

      def get_list(self, ownername, projectname, packagename=None, status=None, pagination=None):
 
@@ -77,8 +73,7 @@ 
 

          }
 

          params.update(pagination or {})
 

  

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def cancel(self, build_id):
 
@@ -89,8 +84,8 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/cancel/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST)
 

          return munchify(response)
 

  

      def create_from_urls(self, ownername, projectname, urls, buildopts=None, project_dirname=None):
 
@@ -296,20 +291,21 @@ 
 

      def _create(self, endpoint, data, files=None, buildopts=None):
 

          data = data.copy()
 

  

-         request_class = Request
 

-         kwargs = {"endpoint": endpoint, "api_base_url": self.api_base_url,
 

-                   "data": data,"method": POST, "auth": self.auth}
 

-         if files:
 

-             request_class = FileRequest
 

-             kwargs["files"] = files
 

- 

+         kwargs = {"api_base_url": self.api_base_url}
 

          if files and buildopts and "progress_callback" in buildopts:
 

              kwargs["progress_callback"] = buildopts["progress_callback"]
 

              del buildopts["progress_callback"]
 

  

          data.update(buildopts or {})
 

-         request = request_class(**kwargs)
 

-         response = request.send()
 

+         if not files:
 

+             self.request.auth = self.auth
 

+             response = self.request.send(endpoint=endpoint, data=data, method=POST)
 

+         else:
 

+             kwargs["files"] = files
 

+             kwargs["auth"] = self.auth
 

+             kwargs["connection_attempts"] = self.config.get("connection_attempts", 1)
 

+             request = FileRequest(**kwargs)
 

+             response = request.send(endpoint=endpoint, data=data, method=POST)
 

          return munchify(response)
 

  

      def delete(self, build_id):
 
@@ -320,8 +316,8 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/build/delete/{0}".format(build_id)
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST)
 

          return munchify(response)
 

  

      def delete_list(self, build_ids):
 
@@ -334,6 +330,6 @@ 
 

  

          endpoint = "/build/delete/list"
 

          data = {"builds": build_ids}
 

-         request = Request(endpoint, data=data, api_base_url=self.api_base_url, method=POST, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, data=data, method=POST)
 

          return munchify(response)

file modified
+5 -9 
@@ -1,7 +1,7 @@ 
 

  from __future__ import absolute_import
 

  

  from . import BaseProxy
 

- from ..requests import Request, munchify
 

+ from ..requests import munchify
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -20,8 +20,7 @@ 
 

              "build_id": build_id,
 

              "chrootname": chrootname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_list(self, build_id, pagination=None):
 
@@ -38,8 +37,7 @@ 
 

              "build_id": build_id,
 

          }
 

          params.update(pagination or {})
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_build_config(self, build_id, chrootname):
 
@@ -55,8 +53,7 @@ 
 

              "build_id": build_id,
 

              "chrootname": chrootname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_built_packages(self, build_id, chrootname):
 
@@ -72,6 +69,5 @@ 
 

              "build_id": build_id,
 

              "chrootname": chrootname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)

file modified
+3 -4 
@@ -1,8 +1,7 @@ 
 

  from __future__ import absolute_import
 

  

- import os
 

  from . import BaseProxy
 

- from ..requests import Request, munchify
 

+ from ..requests import munchify
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -10,11 +9,11 @@ 
 

  class MockChrootProxy(BaseProxy):
 

  

      def get_list(self, pagination=None):
 

+         # TODO: implement pagination
 

          """List all currently available chroots.
 

  

          :return: Munch
 

          """
 

          endpoint = "/mock-chroots/list"
 

-         request = Request(endpoint, api_base_url=self.api_base_url)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint)
 

          return munchify(response)

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

  

  import os
 

  from . import BaseProxy
 

- from ..requests import Request, FileRequest, munchify, POST
 

+ from ..requests import FileRequest, munchify, POST
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -31,9 +31,8 @@ 
 

          }
 

          if distgit is not None:
 

              data["distgit"] = distgit
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def build_from_file(self, ownername, projectname, path, distgit=None):
 
@@ -57,7 +56,7 @@ 
 

          data = None
 

          if distgit is not None:
 

              data = {"distgit": distgit}
 

-         request = FileRequest(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                               params=params, files=files, data=data, auth=self.auth)
 

-         response = request.send()
 

+         request = FileRequest(files=files, api_base_url=self.api_base_url, auth=self.auth,
 

+                               connection_attempts=self.config.get("connection_attempts", 1))
 

+         response = request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)

file modified
+2 -3 
@@ -3,7 +3,7 @@ 
 

  """
 

  

  from copr.v3 import proxies
 

- from copr.v3.requests import Request, munchify
 

+ from copr.v3.requests import munchify
 

  from copr.v3.helpers import for_all_methods, bind_proxy
 

  

  
@@ -47,6 +47,5 @@ 
 

              "project_dirname": project_dirname,
 

              "additional_fields[]": additional_fields,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)

file modified
+11 -15 
@@ -1,7 +1,7 @@ 
 

  from __future__ import absolute_import
 

  from . import BaseProxy
 

  from .build import BuildProxy
 

- from ..requests import Request, munchify, POST
 

+ from ..requests import munchify, POST
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -31,8 +31,7 @@ 
 

              "with_latest_build": with_latest_build,
 

              "with_latest_succeeded_build": with_latest_succeeded_build,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_list(self, ownername, projectname, pagination=None,
 
@@ -59,8 +58,7 @@ 
 

          }
 

          params.update(pagination or {})
 

  

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def add(self, ownername, projectname, packagename, source_type, source_dict):
 
@@ -85,9 +83,8 @@ 
 

              "package_name": packagename,
 

          }
 

          data.update(source_dict)
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def edit(self, ownername, projectname, packagename, source_type=None, source_dict=None):
 
@@ -112,9 +109,8 @@ 
 

              "package_name": packagename,
 

          }
 

          data.update(source_dict or {})
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def reset(self, ownername, projectname, packagename):
 
@@ -134,8 +130,8 @@ 
 

              "projectname": projectname,
 

              "package_name": packagename,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, data=data, method=POST, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, data=data, method=POST)
 

          return munchify(response)
 

  

      def build(self, ownername, projectname, packagename, buildopts=None, project_dirname=None):
 
@@ -174,6 +170,6 @@ 
 

              "projectname": projectname,
 

              "package_name": packagename,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, data=data, method=POST, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, data=data, method=POST)
 

          return munchify(response)

file modified
+20 -54 
@@ -3,7 +3,7 @@ 
 

  import warnings
 

  

  from . import BaseProxy
 

- from ..requests import Request, munchify, POST, GET, PUT
 

+ from ..requests import munchify, POST, GET, PUT
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -30,8 +30,7 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_list(self, ownername=None, pagination=None):
 
@@ -47,8 +46,7 @@ 
 

              "ownername": ownername,
 

          }
 

          params.update(pagination or {})
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def search(self, query, pagination=None):
 
@@ -64,8 +62,7 @@ 
 

              "query": query,
 

          }
 

          params.update(pagination or {})
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def add(self, ownername, projectname, chroots, description=None, instructions=None, homepage=None,
 
@@ -136,9 +133,8 @@ 
 

  

          _compat_use_bootstrap_container(data, use_bootstrap_container)
 

  

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def edit(self, ownername, projectname, chroots=None, description=None, instructions=None, homepage=None,
 
@@ -207,9 +203,8 @@ 
 

  

          _compat_use_bootstrap_container(data, use_bootstrap_container)
 

  

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def delete(self, ownername, projectname):
 
@@ -228,9 +223,8 @@ 
 

          data = {
 

              "verify": True,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def fork(self, ownername, projectname, dstownername, dstprojectname, confirm=False):
 
@@ -255,9 +249,8 @@ 
 

              "ownername": dstownername,
 

              "confirm": confirm,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, data=data, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)
 

  

      def get_permissions(self, ownername, projectname):
 
@@ -277,14 +270,8 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         request = Request(
 

-                 endpoint,
 

-                 api_base_url=self.api_base_url,
 

-                 auth=self.auth,
 

-                 method=GET,
 

-                 params=params)
 

- 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def set_permissions(self, ownername, projectname, permissions):
 
@@ -312,15 +299,8 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         request = Request(
 

-                 endpoint,
 

-                 api_base_url=self.api_base_url,
 

-                 auth=self.auth,
 

-                 method=PUT,
 

-                 params=params,
 

-                 data=permissions)
 

- 

-         request.send()
 

+         self.request.auth = self.auth
 

+         self.request.send(endpoint=endpoint, method=PUT, params=params, data=permissions)
 

  

      def request_permissions(self, ownername, projectname, permissions):
 

          """
 
@@ -341,15 +321,8 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         request = Request(
 

-                 endpoint,
 

-                 api_base_url=self.api_base_url,
 

-                 auth=self.auth,
 

-                 method=PUT,
 

-                 params=params,
 

-                 data=permissions)
 

- 

-         request.send()
 

+         self.request.auth = self.auth
 

+         self.request.send(endpoint=endpoint, method=PUT, params=params, data=permissions)
 

  

      def regenerate_repos(self, ownername, projectname):
 

          """
 
@@ -363,13 +336,6 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname
 

          }
 

-         request = Request(
 

-                 endpoint,
 

-                 api_base_url=self.api_base_url,
 

-                 auth=self.auth,
 

-                 method=PUT,
 

-                 params=params)
 

- 

-         request.send()
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=PUT, params=params)
 

          return munchify(response)

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

  

  import os
 

  from . import BaseProxy
 

- from ..requests import Request, FileRequest, munchify, POST
 

+ from ..requests import FileRequest, munchify, POST
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -24,8 +24,7 @@ 
 

              "projectname": projectname,
 

              "chrootname": chrootname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      def get_build_config(self, ownername, projectname, chrootname):
 
@@ -43,8 +42,7 @@ 
 

              "projectname": projectname,
 

              "chrootname": chrootname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, params=params)
 

-         response = request.send()
 

+         response = self.request.send(endpoint=endpoint, params=params)
 

          return munchify(response)
 

  

      # pylint: disable=too-many-arguments
 
@@ -103,7 +101,7 @@ 
 

              comps_f = open(comps, "rb")
 

              files["upload_comps"] = (os.path.basename(comps_f.name), comps_f, "application/text")
 

  

-         request = FileRequest(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                               params=params, data=data, files=files, auth=self.auth)
 

-         response = request.send()
 

+         request = FileRequest(api_base_url=self.api_base_url, files=files, auth=self.auth,
 

+                               connection_attempts=self.config.get("connection_attempts", 1))
 

+         response = request.send(endpoint=endpoint, method=POST, params=params, data=data)
 

          return munchify(response)

file modified
+3 -4 
@@ -5,7 +5,7 @@ 
 

  from __future__ import absolute_import
 

  

  from . import BaseProxy
 

- from ..requests import Request, munchify, POST
 

+ from ..requests import munchify, POST
 

  from ..helpers import for_all_methods, bind_proxy
 

  

  
@@ -29,7 +29,6 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         request = Request(endpoint, api_base_url=self.api_base_url, method=POST,
 

-                           params=params, auth=self.auth)
 

-         response = request.send()
 

+         self.request.auth = self.auth
 

+         response = self.request.send(endpoint=endpoint, method=POST, params=params)
 

          return munchify(response)

file modified
+38 -14 
@@ -2,6 +2,7 @@ 
 

  

  import os
 

  import json
 

+ import time
 

  import requests
 

  import requests_gssapi
 

  from copr.v3.helpers import List
 
@@ -19,8 +20,9 @@ 
 

  class Request(object):
 

      # This should be a replacement of the _fetch method from APIv1
 

      # We can have Request, FileRequest, AuthRequest/UnAuthRequest, ...
 

+     # pylint: disable=too-many-instance-attributes
 

  

-     def __init__(self, endpoint, api_base_url=None, method=None, data=None, params=None, auth=None):
 

+     def __init__(self, api_base_url=None, auth=None, connection_attempts=1):
 

          """
 

          :param endpoint:
 

          :param api_base_url:
 
@@ -32,12 +34,13 @@ 
 

          @TODO maybe don't have both params and data, but rather only one variable
 

          @TODO and send it as data on POST and as params on GET
 

          """
 

-         self.endpoint = endpoint
 

          self.api_base_url = api_base_url
 

-         self._method = method or GET
 

-         self.data = data
 

-         self.params = params or {}
 

          self.auth = auth
 

+         self.connection_attempts = connection_attempts
 

+         self._method = GET
 

+         self.endpoint = None
 

+         self.data = None
 

+         self.params = {}
 

          self.headers = None
 

  

      @property
 
@@ -49,21 +52,42 @@ 
 

      def method(self):
 

          return self._method.upper()
 

  

-     def send(self):
 

+     def send(self, endpoint, method=GET, data=None, params=None, headers=None):
 

+         if params is None:
 

+             self.params = {}
 

+         else:
 

+             self.params = params
 

+         self.endpoint = endpoint
 

+         self._method = method
 

+         self.data = data
 

+         self.headers = headers
 

+ 

          session = requests.Session()
 

          if not isinstance(self.auth, tuple):
 

              # api token not available, set session cookie obtained via gssapi
 

              session.cookies.set("session", self.auth)
 

  

-         try:
 

-             response = session.request(**self._request_params)
 

-         except requests.exceptions.ConnectionError:
 

-             raise CoprRequestException("Unable to connect to {0}.".format(self.api_base_url))
 

-         except requests_gssapi.exceptions.SPNEGOExchangeError as e:
 

-             raise_from(CoprGssapiException("Kerberos ticket has expired."), e)
 

+         response = self._send_request_repeatedly(session)
 

          handle_errors(response)
 

          return response
 

  

+     def _send_request_repeatedly(self, session):
 

+         """
 

+         Repeat the request until it succeeds, or connection retry reaches its limit.
 

+         """
 

+         sleep = 5
 

+         for i in range(1, self.connection_attempts + 1):
 

+             try:
 

+                 response = session.request(**self._request_params)
 

+             except requests_gssapi.exceptions.SPNEGOExchangeError as e:
 

+                 raise_from(CoprGssapiException("Kerberos ticket has expired."), e)
 

+             except requests.exceptions.ConnectionError:
 

+                 if i < self.connection_attempts:
 

+                     time.sleep(sleep)
 

+             else:
 

+                 return response
 

+         raise CoprRequestException("Unable to connect to {0}.".format(self.api_base_url))
 

+ 

      @property
 

      def _request_params(self):
 

          params = {
 
@@ -81,8 +105,8 @@ 
 

  

  

  class FileRequest(Request):
 

-     def __init__(self, endpoint, files=None, progress_callback=None, **kwargs):
 

-         super(FileRequest, self).__init__(endpoint, **kwargs)
 

+     def __init__(self, files=None, progress_callback=None, **kwargs):
 

+         super(FileRequest, self).__init__(**kwargs)
 

          self.files = files
 

          self.progress_callback = progress_callback

We should try to connect to the frontend more than just one time.
Maybe the apache is restarting. This was causing the command watch-build
to fail immediately and it was returning 1 even though a build succeeded at
the end.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 220cc17fd072a7f5c23b4ef26b95731d9477be7f
2 years ago

The ConnectionError is probably never raised?

Build succeeded.

The get_build_details_counter should be reset anytime we receive a valid answer from the server IMO.

But eventually, could we add this re-try logic into the build_proxy.get() method instead?
Something like get(build_id=build_id, retry_connection=3)?

Unfortunately, the Request() class is somewhat decoupled from all the proxies,
and the Python API isn't ready to allow us to specify this additional option into
all the methods. Ideas? @frostyx ?

I'd prefer to change the class SafeRequest to accept retry_connection, now in _send_request_repeatedly it only works with try_indefinitely and as an opposite to this, a user could set how many times an endpoint should be contacted before the request fails.

rebased onto f8a9a808b587f8d7496fb07a3d88b09228c41884
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Seems like this is not yet ready for review? Per our meeting, we should be able to replace Request() with some wrapper everywhere, and be able to propagate the self.config["connection_retry"] automatically to every Request instance.

This is waiting on @forstyx.

This sleep is done even after the last request, before throwing the final exception. We should fix that.

we should be able to replace Request() with some wrapper everywhere

I have two suggestions on how would I go implementing it.
The lazy way would be adding the following method to the BaseProxy

def make_request(self, endpoint, method=None, data=None, params=None):
    # We don't take things that we know from `self` as arguments, e.g.
    # `self.auth`, `self.config["connection_retry"]`, etc
    return Request(endpoint, ...)

Then everywhere in the code where we do 

request = Request(endpoint, ...)

we would instead do

request = self.make_request(endpoint, ...)

There is also a FileRequest for uploading files but I would leave it
as-is.

The slightly more complicated but IMHO prettier solution would be the
following. In retrospect, I think I designed the Request class a bit
unfortunately. 

class Request(object):
    def __init__(self, endpoint, api_base_url=None, method=None, data=None, params=None, auth=None, connection_retry=1):
        ...

    def send(self):
        ...

The idea was to be able to prepare the request on one place and send
it when necessary. It's a feature that would have been useful in the
situation that we are dealing with now, if implemented properly.

I think we should initialize Request only with the common
parameters, such as api_base_url, auth, and
connection_retry. Then move the rest of the parameters to send
method.

class Request(object):
    def __init__(self, api_base_url=None, auth=None, connection_retry=1):
        ...

    def send(self, endpoint, method=None, data=None, params=None):
        ...

Then I would do this:

class BaseProxy(object):
    def __init__(self, config):
        ...
        self.request = Request(...)

and everywhere where we use

request = Request(endpoint, ...)
response = request.send()

we would use this instead

response = self.request.send(...)

What do you think?

I'm OK with both, the second is probably more optimized as the request isn't
re-initiated all the time. WRT GSSAPI PR #1820, both are OK IMO -> we can
easily replace the requests module with requests.Session() object
on one place.

Thanks for the suggestions :)
I probably like the second one more so I'll implement that one.

rebased onto 52f4803f5e1598abc538c67203c18d7c8f2ea697
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto c42745fec7beb7e8122ef92e52d4fc9b358cfbf3
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 719bc8efe7b60203006703b31c0042d7810edecb
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

@frostyx PTAL

Nice and easy. Maybe a nit, can we name it like "connection_attempts" or "connection_retries" ... the name sounds like it was a boolean value.

I would probably move this into the Request() constructor, as proposed by @frostyx:

Request(api_base_url=..., connection_retry=config.get("connection_retry")

By any chance ... are users using the Request class directly? Because that would be an API change, @frostyx?
Perhaps task for the future -- make sure the non-public things are all in copr._private or something.

I don't think so. Class Request is not in python/copr/v3/__init__.py

rebased onto 3988604bbdf19283509ee18715385b1631b6da43
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

By any chance ... are users using the Request class directly? Because that would be an API change, @frostyx?
I don't think so. Class Request is not in python/copr/v3/init.py

I don't think so either. It is not even documented here
https://python-copr.readthedocs.io/en/latest/ClientV3.html

Perhaps task for the future -- make sure the non-public things are all in copr._private or something.

We can do something like that, yes :-)

I think we shouldn't do it this way because this will set the auth not only for the request but for every other request that comes after this. So sometimes for example ProjectProxy.get request would be sent without auth and sometimes with auth, depending on what requests were done before that.

I think we should either set the request.auth in BaseProxy.__init__ or we should do request.send(auth=self.auth). I think I would prefer the first option.

This is an API change though.
I know the parameter isn't used and there most likely nobody is using it. But I would probably put a # TODO implement pagination rather than removing the parameter. Or a comment that the parameter is deprecated and will be removed sometime in the future. Depending on what we want.

Nit: method=GET is the default, we can omit the parameter.

I know it wasn't introduced by this PR but since we are here ... does anybody understand why we send the same request twice? In case it is necessary, it would be a good idea to put some comment to the code because it is really confusing. Otherwise, we should drop it.

Sounds like a candidate for removal, yes.

I think we shouldn't configure the self parameters here and just send the request. I understand, why you do it this way - _send_request_repeatedly _request_params function uses them internally, and you can't really get rid of the function because FileRequest depends on it.

I will try to come up with some follow-up PR and change this part of the code a little bit more. I just don't know how, yet.
Edited 2 years ago by frostyx

Good catch, I propose fixing this right away... , or at least have an issue with a concrete assignee to take care of this.

rebased onto 457062c11a212b52203cf47dc428fae489377296
2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

The issue here https://pagure.io/copr/copr/issue/2138. After a discussion with Jakub, we agreed to open a new issue and he will look at it and try to come up with something.

Metadata Update from @praiskup:
- Request assigned
2 years ago

LGTM,
Merging so I can work on #2138.

rebased onto ea06ca5
2 years ago

Pull-Request has been merged by frostyx
2 years ago
Metadata
No Tags
Changes Summary 16
+1 -0
file changed
cli/copr_cli/main.py
+5 -5
file changed
python/copr/test/client_v3/test_builds.py
+3 -2
file changed
python/copr/test/client_v3/test_helpers.py
+2 -2
file changed
python/copr/test/client_v3/test_packages.py
+7 -6
file changed
python/copr/test/client_v3/test_requests.py
+4 -4
file changed
python/copr/v3/proxies/__init__.py
+22 -26
file changed
python/copr/v3/proxies/build.py
+5 -9
file changed
python/copr/v3/proxies/build_chroot.py
+3 -4
file changed
python/copr/v3/proxies/mock_chroot.py
+6 -7
file changed
python/copr/v3/proxies/module.py
+2 -3
file changed
python/copr/v3/proxies/monitor.py
+11 -15
file changed
python/copr/v3/proxies/package.py
+20 -54
file changed
python/copr/v3/proxies/project.py
+6 -8
file changed
python/copr/v3/proxies/project_chroot.py
+3 -4
file changed
python/copr/v3/proxies/webhook.py
+38 -14
file changed
python/copr/v3/requests.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.