From 6251539dcecfe9fe0e7fbd7be1ca66477e163fa6 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Mon, 7 Aug 2023 19:08:31 +0100 Subject: [PATCH] proxy-server: de-duplicate implementation of is_good_source Both the EC and replicated GET paths have is_good_source() methods that are similar. Refactor to have just one implementation. Change-Id: I661d3704a76e3d92bfcfeaed1fff4ed5e28c79b4 --- swift/proxy/controllers/base.py | 32 ++++++++++++++++++-------------- swift/proxy/controllers/obj.py | 22 +++++----------------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index bec4b7e1ab..756484df29 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -53,7 +53,8 @@ from swift.common.header_key_dict import HeaderKeyDict from swift.common.http import is_informational, is_success, is_redirection, \ is_server_error, HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_MULTIPLE_CHOICES, \ HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_SERVICE_UNAVAILABLE, \ - HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE + HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE, \ + HTTP_REQUESTED_RANGE_NOT_SATISFIABLE from swift.common.swob import Request, Response, Range, \ HTTPException, HTTPRequestedRangeNotSatisfiable, HTTPServiceUnavailable, \ status_map, wsgi_to_str, str_to_wsgi, wsgi_quote, wsgi_unquote, \ @@ -1017,6 +1018,21 @@ def bytes_to_skip(record_size, range_start): return (record_size - (range_start % record_size)) % record_size +def is_good_source(status, server_type): + """ + Indicates whether or not the request made to the backend found + what it was looking for. + + :param resp: the response from the backend. + :param server_type: the type of server: 'Account', 'Container' or 'Object'. + :returns: True if the response status code is acceptable, False if not. + """ + if (server_type == 'Object' and + status == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE): + return True + return is_success(status) or is_redirection(status) + + class ByteCountEnforcer(object): """ Enforces that successive calls to file_like.read() give at least @@ -1260,18 +1276,6 @@ class GetOrHeadHandler(GetterBase): # populated from response headers self.start_byte = self.end_byte = self.length = None - def _is_good_source(self, src): - """ - Indicates whether or not the request made to the backend found - what it was looking for. - - :param src: the response from the backend - :returns: True if found, False if not - """ - if self.server_type == 'Object' and src.status == 416: - return True - return is_success(src.status) or is_redirection(src.status) - def _get_next_response_part(self): # return the next part of the response body; there may only be one part # unless it's a multipart/byteranges response @@ -1448,7 +1452,7 @@ class GetOrHeadHandler(GetterBase): src_headers = dict( (k.lower(), v) for k, v in possible_source.getheaders()) - if self._is_good_source(possible_source): + if is_good_source(possible_source.status, self.server_type): # 404 if we know we don't have a synced copy if not float(possible_source.getheader('X-PUT-Timestamp', 1)): self.statuses.append(HTTP_NOT_FOUND) diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index f93917c131..bd18bf0993 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -69,7 +69,8 @@ from swift.common.storage_policy import (POLICIES, REPL_POLICY, EC_POLICY, ECDriverError, PolicyError) from swift.proxy.controllers.base import Controller, delay_denial, \ cors_validation, update_headers, bytes_to_skip, ByteCountEnforcer, \ - record_cache_op_metrics, get_cache_key, GetterBase, GetterSource + record_cache_op_metrics, get_cache_key, GetterBase, GetterSource, \ + is_good_source from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \ HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \ HTTPServerError, HTTPServiceUnavailable, HTTPClientDisconnect, \ @@ -2477,19 +2478,6 @@ class ECGetResponseCollection(object): return nodes.pop(0).copy() -def is_good_source(status): - """ - Indicates whether or not the request made to the backend found - what it was looking for. - - :param status: the response from the backend - :returns: True if found, False if not - """ - if status == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE: - return True - return is_success(status) or is_redirection(status) - - class ECFragGetter(GetterBase): def __init__(self, app, req, node_iter, partition, policy, path, @@ -2714,7 +2702,7 @@ class ECFragGetter(GetterBase): self.status = possible_source.status self.reason = possible_source.reason self.source_headers = possible_source.getheaders() - if is_good_source(possible_source.status): + if is_good_source(possible_source.status, server_type='Object'): self.body = None return possible_source else: @@ -2945,9 +2933,9 @@ class ECObjectController(BaseObjectController): break requests_available = extra_requests < max_extra_requests and ( node_iter.nodes_left > 0 or buckets.has_alternate_node()) - bad_resp = not is_good_source(get.last_status) if requests_available and ( - buckets.shortfall > pile._pending or bad_resp): + buckets.shortfall > pile._pending or + not is_good_source(get.last_status, self.server_type)): extra_requests += 1 pile.spawn(self._fragment_GET_request, req, safe_iter, partition, policy, buckets.get_extra_headers,