From b107de072b9aacc03a22e2f7ea7f8efeb3c7f41b Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Fri, 19 Jan 2024 09:37:33 +0100 Subject: [PATCH] oci: use pickleable errors (#42160) --- lib/spack/spack/cmd/buildcache.py | 5 +++-- lib/spack/spack/oci/oci.py | 37 ++++++++++++++----------------- lib/spack/spack/oci/opener.py | 30 +++++++++++-------------- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py index ddfdee2907..3f4f040659 100644 --- a/lib/spack/spack/cmd/buildcache.py +++ b/lib/spack/spack/cmd/buildcache.py @@ -716,8 +716,9 @@ def _config_from_tag(image_ref: ImageReference, tag: str) -> Optional[dict]: def _update_index_oci(image_ref: ImageReference, tmpdir: str, pool: MaybePool) -> None: - response = spack.oci.opener.urlopen(urllib.request.Request(url=image_ref.tags_url())) - spack.oci.opener.ensure_status(response, 200) + request = urllib.request.Request(url=image_ref.tags_url()) + response = spack.oci.opener.urlopen(request) + spack.oci.opener.ensure_status(request, response, 200) tags = json.load(response)["tags"] # Fetch all image config files in parallel diff --git a/lib/spack/spack/oci/oci.py b/lib/spack/spack/oci/oci.py index 4e5e196cd1..0af68f3340 100644 --- a/lib/spack/spack/oci/oci.py +++ b/lib/spack/spack/oci/oci.py @@ -134,7 +134,7 @@ def upload_blob( return True # Otherwise, do another PUT request. - spack.oci.opener.ensure_status(response, 202) + spack.oci.opener.ensure_status(request, response, 202) assert "Location" in response.headers # Can be absolute or relative, joining handles both @@ -143,19 +143,16 @@ def upload_blob( ) f.seek(0) - response = _urlopen( - Request( - url=upload_url, - method="PUT", - data=f, - headers={ - "Content-Type": "application/octet-stream", - "Content-Length": str(file_size), - }, - ) + request = Request( + url=upload_url, + method="PUT", + data=f, + headers={"Content-Type": "application/octet-stream", "Content-Length": str(file_size)}, ) - spack.oci.opener.ensure_status(response, 201) + response = _urlopen(request) + + spack.oci.opener.ensure_status(request, response, 201) # print elapsed time and # MB/s _log_upload_progress(digest, file_size, time.time() - start) @@ -189,16 +186,16 @@ def upload_manifest( if not tag: ref = ref.with_digest(digest) - response = _urlopen( - Request( - url=ref.manifest_url(), - method="PUT", - data=data, - headers={"Content-Type": oci_manifest["mediaType"]}, - ) + request = Request( + url=ref.manifest_url(), + method="PUT", + data=data, + headers={"Content-Type": oci_manifest["mediaType"]}, ) - spack.oci.opener.ensure_status(response, 201) + response = _urlopen(request) + + spack.oci.opener.ensure_status(request, response, 201) return digest, size diff --git a/lib/spack/spack/oci/opener.py b/lib/spack/spack/oci/opener.py index 792598578d..6ee8ecffbd 100644 --- a/lib/spack/spack/oci/opener.py +++ b/lib/spack/spack/oci/opener.py @@ -310,19 +310,15 @@ def http_error_401(self, req: Request, fp, code, msg, headers): # Login failed, avoid infinite recursion where we go back and # forth between auth server and registry if hasattr(req, "login_attempted"): - raise urllib.error.HTTPError( - req.full_url, code, f"Failed to login to {req.full_url}: {msg}", headers, fp + raise spack.util.web.DetailedHTTPError( + req, code, f"Failed to login: {msg}", headers, fp ) # On 401 Unauthorized, parse the WWW-Authenticate header # to determine what authentication is required if "WWW-Authenticate" not in headers: - raise urllib.error.HTTPError( - req.full_url, - code, - "Cannot login to registry, missing WWW-Authenticate header", - headers, - fp, + raise spack.util.web.DetailedHTTPError( + req, code, "Cannot login to registry, missing WWW-Authenticate header", headers, fp ) header_value = headers["WWW-Authenticate"] @@ -330,8 +326,8 @@ def http_error_401(self, req: Request, fp, code, msg, headers): try: challenge = get_bearer_challenge(parse_www_authenticate(header_value)) except ValueError as e: - raise urllib.error.HTTPError( - req.full_url, + raise spack.util.web.DetailedHTTPError( + req, code, f"Cannot login to registry, malformed WWW-Authenticate header: {header_value}", headers, @@ -340,8 +336,8 @@ def http_error_401(self, req: Request, fp, code, msg, headers): # If there is no bearer challenge, we can't handle it if not challenge: - raise urllib.error.HTTPError( - req.full_url, + raise spack.util.web.DetailedHTTPError( + req, code, f"Cannot login to registry, unsupported authentication scheme: {header_value}", headers, @@ -356,8 +352,8 @@ def http_error_401(self, req: Request, fp, code, msg, headers): timeout=req.timeout, ) except ValueError as e: - raise urllib.error.HTTPError( - req.full_url, + raise spack.util.web.DetailedHTTPError( + req, code, f"Cannot login to registry, failed to obtain bearer token: {e}", headers, @@ -412,13 +408,13 @@ def create_opener(): return opener -def ensure_status(response: HTTPResponse, status: int): +def ensure_status(request: urllib.request.Request, response: HTTPResponse, status: int): """Raise an error if the response status is not the expected one.""" if response.status == status: return - raise urllib.error.HTTPError( - response.geturl(), response.status, response.reason, response.info(), None + raise spack.util.web.DetailedHTTPError( + request, response.status, response.reason, response.info(), None )