copr / copr

Clone

#2149 Refactor copr.v3.Request attributes
Merged 2 years ago by praiskup. Opened 2 years ago by frostyx.
copr/ frostyx/copr python-send-refactor  into  main

file modified
+2 -4 
@@ -20,13 +20,11 @@ 
 

  class TestRequest(object):
 

      def test_endpoint_url(self):
 

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

-         r1.endpoint = "foo"
 

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

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

  

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

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

-         r2.endpoint = "/foo/bar"
 

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

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

  

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

      def test_send(self, request):

file modified
+1 -2 
@@ -134,8 +134,7 @@ 
 

          :return: Munch
 

          """
 

          endpoint = "/auth-check"
 

-         self.request.auth = self.auth
 

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

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

          return munchify(response)

file modified
+10 -10 
@@ -84,8 +84,8 @@ 
 

          :return: Munch
 

          """
 

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

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, method=POST, auth=self.auth)
 

          return munchify(response)
 

  

      def create_from_urls(self, ownername, projectname, urls, buildopts=None, project_dirname=None):
 
@@ -298,14 +298,14 @@ 
 

  

          data.update(buildopts or {})
 

          if not files:
 

-             self.request.auth = self.auth
 

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

+             response = self.request.send(
 

+                 endpoint=endpoint, data=data, method=POST, auth=self.auth)
 

          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)
 

+             response = request.send(
 

+                 endpoint=endpoint, data=data, method=POST, auth=self.auth)
 

          return munchify(response)
 

  

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

          :return: Munch
 

          """
 

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

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, method=POST, auth=self.auth)
 

          return munchify(response)
 

  

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

  

          endpoint = "/build/delete/list"
 

          data = {"builds": build_ids}
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, data=data, method=POST, auth=self.auth)
 

          return munchify(response)

file modified
+19 -5 
@@ -31,8 +31,13 @@ 
 

          }
 

          if distgit is not None:
 

              data["distgit"] = distgit
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

          data = None
 

          if distgit is not None:
 

              data = {"distgit": distgit}
 

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

+         request = FileRequest(
 

+             files=files,
 

+             api_base_url=self.api_base_url,
 

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

+         )
 

+         response = request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)

file modified
+18 -8 
@@ -83,8 +83,13 @@ 
 

              "package_name": packagename,
 

          }
 

          data.update(source_dict)
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

              "package_name": packagename,
 

          }
 

          data.update(source_dict or {})
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

              "projectname": projectname,
 

              "package_name": packagename,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, data=data, method=POST, auth=self.auth)
 

          return munchify(response)
 

  

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

              "projectname": projectname,
 

              "package_name": packagename,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, data=data, method=POST, auth=self.auth)
 

          return munchify(response)

file modified
+46 -16 
@@ -133,8 +133,13 @@ 
 

  

          _compat_use_bootstrap_container(data, use_bootstrap_container)
 

  

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

  

          _compat_use_bootstrap_container(data, use_bootstrap_container)
 

  

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

      def delete(self, ownername, projectname):
 
@@ -223,8 +233,13 @@ 
 

          data = {
 

              "verify": True,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

              "ownername": dstownername,
 

              "confirm": confirm,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)
 

  

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

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, params=params, auth=self.auth)
 

          return munchify(response)
 

  

      def set_permissions(self, ownername, projectname, permissions):
 
@@ -299,8 +319,13 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         self.request.auth = self.auth
 

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

+         self.request.send(
 

+             endpoint=endpoint,
 

+             method=PUT,
 

+             params=params,
 

+             data=permissions,
 

+             auth=self.auth,
 

+         )
 

  

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

          """
 
@@ -321,8 +346,13 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         self.request.auth = self.auth
 

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

+         self.request.send(
 

+             endpoint=endpoint,
 

+             method=PUT,
 

+             params=params,
 

+             data=permissions,
 

+             auth=self.auth,
 

+         )
 

  

      def regenerate_repos(self, ownername, projectname):
 

          """
 
@@ -336,6 +366,6 @@ 
 

              "ownername": ownername,
 

              "projectname": projectname
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, method=PUT, params=params, auth=self.auth)
 

          return munchify(response)

file modified
+12 -3 
@@ -101,7 +101,16 @@ 
 

              comps_f = open(comps, "rb")
 

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

  

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

+         request = FileRequest(
 

+             api_base_url=self.api_base_url,
 

+             files=files,
 

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

+         )
 

+         response = request.send(
 

+             endpoint=endpoint,
 

+             method=POST,
 

+             params=params,
 

+             data=data,
 

+             auth=self.auth,
 

+         )
 

          return munchify(response)

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

              "ownername": ownername,
 

              "projectname": projectname,
 

          }
 

-         self.request.auth = self.auth
 

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

+         response = self.request.send(
 

+             endpoint=endpoint, method=POST, params=params, auth=self.auth)
 

          return munchify(response)

file modified
+28 -49 
@@ -20,65 +20,45 @@ 
 

  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, api_base_url=None, auth=None, connection_attempts=1):
 

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

          """
 

-         :param endpoint:
 

          :param api_base_url:
 

-         :param method:
 

-         :param data: dict
 

-         :param params: dict for constructing query params in URL (e.g. ?key1=val1)
 

-         :param auth: tuple (login, token)
 

+         :param connection_attempts:
 

  

          @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.api_base_url = api_base_url
 

-         self.auth = auth
 

          self.connection_attempts = connection_attempts
 

-         self._method = GET
 

-         self.endpoint = None
 

-         self.data = None
 

-         self.params = {}
 

-         self.headers = None
 

- 

-     @property
 

-     def endpoint_url(self):
 

-         endpoint = self.endpoint.strip("/").format(**self.params)
 

-         return os.path.join(self.api_base_url, endpoint)
 

- 

-     @property
 

-     def method(self):
 

-         return self._method.upper()
 

  

-     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
 

+     def endpoint_url(self, endpoint, params=None):
 

+         params = params or {}
 

+         endpoint = endpoint.strip("/").format(**params)
 

+         return os.path.join(self.api_base_url, endpoint)
 

  

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

+              auth=None):
 

          session = requests.Session()
 

-         if not isinstance(self.auth, tuple):
 

+         if not isinstance(auth, tuple):
 

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

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

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

+ 

+         request_params = self._request_params(
 

+             endpoint, method, data, params, headers, auth)
 

  

-         response = self._send_request_repeatedly(session)
 

+         response = self._send_request_repeatedly(session, request_params)
 

          handle_errors(response)
 

          return response
 

  

-     def _send_request_repeatedly(self, session):
 

+     def _send_request_repeatedly(self, session, request_params):
 

          """
 

          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)
 

+                 response = session.request(**request_params)
 

              except requests_gssapi.exceptions.SPNEGOExchangeError as e:
 

                  raise_from(CoprAuthException("GSSAPI authentication failed."), e)
 

              except requests.exceptions.ConnectionError:
 
@@ -88,19 +68,19 @@ 
 

                  return response
 

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

  

-     @property
 

-     def _request_params(self):
 

+     def _request_params(self, endpoint, method=GET, data=None, params=None,
 

+                         headers=None, auth=None):
 

          params = {
 

-             "url": self.endpoint_url,
 

-             "json": self.data,
 

-             "method": self.method,
 

-             "params": self.params,
 

-             "headers": self.headers,
 

+             "url": self.endpoint_url(endpoint, params),
 

+             "json": data,
 

+             "method": method.upper(),
 

+             "params": params,
 

+             "headers": headers,
 

          }
 

          # We usually use a tuple (login, token). If this is not available,
 

          # we use gssapi auth, which works with cookies.
 

-         if isinstance(self.auth, tuple):
 

-             params["auth"] = self.auth
 

+         if isinstance(auth, tuple):
 

+             params["auth"] = auth
 

          return params
 

  

  
@@ -110,12 +90,11 @@ 
 

          self.files = files
 

          self.progress_callback = progress_callback
 

  

-     @property
 

-     def _request_params(self):
 

-         params = super(FileRequest, self)._request_params
 

+     def _request_params(self, *args, **kwargs):
 

+         params = super(FileRequest, self)._request_params(*args, **kwargs)
 

  

          data = self.files or {}
 

-         data["json"] = ("json", json.dumps(self.data), "application/json")
 

+         data["json"] = ("json", json.dumps(params["json"]), "application/json")
 

  

          callback = self.progress_callback or (lambda x: x)
 

          m = MultipartEncoder(data)

Fix #2138
See PR#2102

Please see the commit descriptions for more info.

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

I am intentionally ignoring the Zuul error about duplicate code. IMHO we don't want to deduplicate it because it would obscure it too much.

I'm afraid out testsuite is still too poor for such changes :-(

$ copr build ping /tmp/quick-package/dummy-pkg-20220330_0758-1.src.rpm --chroot fedora-rawhide-x86_64
Uploading package /tmp/quick-package/dummy-pkg-20220330_0758-1.src.rpm

Traceback (most recent call last):
  File "/usr/bin/copr", line 33, in <module>
    sys.exit(load_entry_point('copr-cli==1.99', 'console_scripts', 'copr-cli')())
  File "/usr/lib/python3.10/site-packages/copr_cli/main.py", line 1796, in main
    getattr(commands, arg.func)(arg)
  File "/usr/lib/python3.10/site-packages/copr_cli/main.py", line 156, in wrapper
    return func(self, args)
  File "/usr/lib/python3.10/site-packages/copr_cli/main.py", line 327, in action_build
    builds.append(self.process_build(args, build_function, data, bar=bar, progress_callback=progress_callback))
  File "/usr/lib/python3.10/site-packages/copr_cli/main.py", line 416, in process_build
    result = build_function(ownername=username, projectname=projectname,
  File "/usr/lib/python3.10/site-packages/copr/v3/helpers.py", line 69, in wrapper
    result = func(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/copr/v3/proxies/build.py", line 150, in create_from_file
    return self._create(endpoint, data, files=files, buildopts=buildopts)
  File "/usr/lib/python3.10/site-packages/copr/v3/helpers.py", line 69, in wrapper
    result = func(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/copr/v3/proxies/build.py", line 310, in _create
    response = request.send(
  File "/usr/lib/python3.10/site-packages/copr/v3/requests.py", line 47, in send
    request_params = self._request_params(
  File "/usr/lib/python3.10/site-packages/copr/v3/requests.py", line 97, in _request_params
    data["json"] = ("json", json.dumps(params["json"]), "application/json")
  File "/usr/lib64/python3.10/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/lib64/python3.10/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib64/python3.10/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/lib64/python3.10/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type function is not JSON serializable

params["json"] contains progress_callback and json.dumps doesn't know how to deal with it

2 new commits added

  • python: move auth from request init to send
  • python: avoid mutating Request object from within its send method
2 years ago

params["json"] contains progress_callback and json.dumps doesn't know how to deal with it

You are right. I dropped the latest commit that refactored the BuildProxy._create method. It should work now.

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

rebased onto 5488401
2 years ago

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

+1

Commit 52b449b fixes this pull-request

Pull-Request has been merged by praiskup
2 years ago

Commit 5ddb2c7 fixes this pull-request

Pull-Request has been merged by praiskup
2 years ago
Metadata
None
No Tags
Changes Summary 9
+2 -4
file changed
python/copr/test/client_v3/test_requests.py
+1 -2
file changed
python/copr/v3/proxies/__init__.py
+10 -10
file changed
python/copr/v3/proxies/build.py
+19 -5
file changed
python/copr/v3/proxies/module.py
+18 -8
file changed
python/copr/v3/proxies/package.py
+46 -16
file changed
python/copr/v3/proxies/project.py
+12 -3
file changed
python/copr/v3/proxies/project_chroot.py
+2 -2
file changed
python/copr/v3/proxies/webhook.py
+28 -49
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.