From 49cfd020fdba16af2a99705211432adec6b82900 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 28 Apr 2023 14:38:43 +0200 Subject: [PATCH 1/3] Fix 'diff' command to support diffing selected files only --- osc/commandline.py | 3 ++- osc/core.py | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/osc/commandline.py b/osc/commandline.py index c62cdfa21..522c26ead 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -4646,9 +4646,10 @@ def do_diff(self, subcmd, opts, *args): for i in pac.get_diff(rev1): diff += b''.join(i) else: + files = args diff += server_diff_noex(pac.apiurl, pac.prjname, pac.name, rev1, pac.prjname, pac.name, rev2, - not opts.plain, opts.missingok, opts.meta, not opts.unexpand) + not opts.plain, opts.missingok, opts.meta, not opts.unexpand, files=files) run_pager(diff) @cmdln.option('--issues-only', action='store_true', diff --git a/osc/core.py b/osc/core.py index c58213837..d5779dd26 100644 --- a/osc/core.py +++ b/osc/core.py @@ -11,6 +11,7 @@ import codecs +import copy import datetime import difflib import errno @@ -3574,6 +3575,21 @@ def pathjoin(a, *p): return path +def osc_urlencode(data): + """ + An urlencode wrapper that encodes dictionaries in OBS compatible way: + {"file": ["foo", "bar"]} -> &file[]=foo&file[]=bar + """ + data = copy.deepcopy(data) + if isinstance(data, dict): + for key, value in list(data.items()): + if isinstance(value, list): + del data[key] + data[f"{key}[]"] = value + + return urlencode(data, doseq=True) + + def makeurl(baseurl: str, l, query=None): """Given a list of path compoments, construct a complete URL. @@ -3589,7 +3605,7 @@ def makeurl(baseurl: str, l, query=None): if isinstance(query, list): query = '&'.join(query) elif isinstance(query, dict): - query = urlencode(query) + query = osc_urlencode(query) scheme, netloc, path = urlsplit(baseurl)[0:3] return urlunsplit((scheme, netloc, '/'.join([path] + list(l)), query, '')) @@ -5283,6 +5299,7 @@ def server_diff( onlyissues=False, full=True, xml=False, + files: list = None, ): query: Dict[str, Union[str, int]] = {"cmd": "diff"} if expand: @@ -5308,6 +5325,8 @@ def server_diff( query['onlyissues'] = 1 query['view'] = 'xml' query['unified'] = 0 + if files: + query["file"] = files u = makeurl(apiurl, ['source', new_project, new_package], query=query) f = http_POST(u) @@ -5345,12 +5364,13 @@ def server_diff_noex( expand=True, onlyissues=False, xml=False, + files: list = None, ): try: return server_diff(apiurl, old_project, old_package, old_revision, new_project, new_package, new_revision, - unified, missingok, meta, expand, onlyissues, True, xml) + unified, missingok, meta, expand, onlyissues, True, xml, files=files) except HTTPError as e: msg = None body = None @@ -5367,7 +5387,7 @@ def server_diff_noex( rdiff += server_diff_noex(apiurl, old_project, old_package, old_revision, new_project, new_package, new_revision, - unified, missingok, meta, False) + unified, missingok, meta, False, files=files) except: elm = ET.fromstring(body).find('summary') summary = '' From b0629f6b909c7d953267542ed84718d1ee63862c Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 28 Apr 2023 14:53:29 +0200 Subject: [PATCH 2/3] connection: Allow disabling retry on 400 HTTP status code We probably want the retry on by default to retry on running services, but in some cases such as osc diff it only adds delays while handling a quite expected errror state. --- osc/connection.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/osc/connection.py b/osc/connection.py index 4a2667c6b..ba55aaac4 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -135,12 +135,12 @@ def http_request_wrap_file(func): Turn file path into a file object and close it automatically by using a context manager. """ - def new_func(method, url, headers=None, data=None, file=None): + def new_func(method, url, headers=None, data=None, file=None, retry_on_400: bool = True): if file: with open(file, "rb") as f: - return func(method, url, headers, data, file=f) + return func(method, url, headers, data, f, retry_on_400) else: - return func(method, url, headers, data, file) + return func(method, url, headers, data, file, retry_on_400) new_func.__name__ = func.__name__ new_func.__doc__ = func.__doc__ @@ -148,7 +148,7 @@ def new_func(method, url, headers=None, data=None, file=None): @http_request_wrap_file -def http_request(method: str, url: str, headers=None, data=None, file=None): +def http_request(method: str, url: str, headers=None, data=None, file=None, retry_on_400: bool = True): """ Send a HTTP request to a server. @@ -168,6 +168,7 @@ def http_request(method: str, url: str, headers=None, data=None, file=None): :param headers: Dictionary of custom headers to send. :param data: Data to send in the request body (conflicts with `file`). :param file: Path to a file to send as data in the request body (conflicts with `data`). + :param retry_on_400: Whether to retry on receiving HTTP status code 400. """ purl = urllib3.util.parse_url(url) @@ -227,16 +228,22 @@ def http_request(method: str, url: str, headers=None, data=None, file=None): else: retries_kwargs = {"method_whitelist": None} + + status_forcelist = ( + 500, # Internal Server Error + 502, # Bad Gateway + 503, # Service Unavailable + 504, # Gateway Timeout + ) + if retry_on_400: + status_forcelist = ( + 400, # Bad Request; retry on 400: service in progress + ) + status_forcelist + pool_kwargs["retries"] = urllib3.Retry( total=int(conf.config["http_retries"]), backoff_factor=2, - status_forcelist=( - 400, # Bad Request; retry on 400: service in progress - 500, # Internal Server Error - 502, # Bad Gateway - 503, # Service Unavailable - 504, # Gateway Timeout - ), + status_forcelist=status_forcelist, # don't raise because we want an actual response rather than a MaxRetryError with "too many error responses" message raise_on_status=False, **retries_kwargs, From 80fc2bf7cbfda83b1a6ba22fef82576f577ec46a Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 28 Apr 2023 14:54:18 +0200 Subject: [PATCH 3/3] Don't retry on 400 HTTP status code in core.server_diff() --- osc/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osc/core.py b/osc/core.py index d5779dd26..78cdb66d2 100644 --- a/osc/core.py +++ b/osc/core.py @@ -5329,7 +5329,7 @@ def server_diff( query["file"] = files u = makeurl(apiurl, ['source', new_project, new_package], query=query) - f = http_POST(u) + f = http_POST(u, retry_on_400=False) if onlyissues and not xml: del_issue_list = [] add_issue_list = []