From b40a45149a73a6b00e92fd1d2df4c0b1543d0005 Mon Sep 17 00:00:00 2001 From: Michael Moss Date: Tue, 10 Oct 2017 11:07:17 -0700 Subject: [PATCH] Allow specifying alternative "success" codes in gitiles requests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in the release recipes because there are times when we want to check that a file is _not_ there yet (i.e. 404 response). R=agable@chromium.org, dnj@chromium.org Change-Id: Iedf646d6c73ebf898e90afd2009840b1e5c5b1ab Reviewed-on: https://chromium-review.googlesource.com/709916 Reviewed-by: PaweÅ‚ Hajdan Jr. Commit-Queue: Michael Moss --- gerrit_util.py | 8 +++++++- recipes/README.recipes.md | 12 ++++++------ recipes/recipe_modules/gitiles/api.py | 5 +++++ .../gitiles/examples/full.expected/basic.json | 18 ++++++++++++++++++ .../recipe_modules/gitiles/examples/full.py | 6 ++++++ .../gitiles/resources/gerrit_client.py | 17 ++++++++++++----- 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index f13cb8159..c02eb267b 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -358,9 +358,15 @@ def ReadHttpResponse(conn, accept_statuses=frozenset([200])): # If response.status < 500 then the result is final; break retry loop. # If the response is 404, it might be because of replication lag, so # keep trying anyway. - if response.status < 500 and response.status != 404: + if ((response.status < 500 and response.status != 404) + or response.status in accept_statuses): LOGGER.debug('got response %d for %s %s', response.status, conn.req_params['method'], conn.req_params['uri']) + # If 404 was in accept_statuses, then it's expected that the file might + # not exist, so don't return the gitiles error page because that's not the + # "content" that was actually requested. + if response.status == 404: + contents = '' break # A status >=500 is assumed to be a possible transient error; retry. http_version = 'HTTP/%s' % ('1.1' if response.version == 11 else '1.0') diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index c2c2686d0..fa723407d 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -502,7 +502,7 @@ DEPRECATED. Consider using gerrit.get_change_description instead. Module for polling a git repository using the Gitiles web interface. -— **def [commit\_log](/recipes/recipe_modules/gitiles/api.py#97)(self, url, commit, step_name=None, attempts=None):** +— **def [commit\_log](/recipes/recipe_modules/gitiles/api.py#102)(self, url, commit, step_name=None, attempts=None):** Returns: (dict) the Gitiles commit log structure for a given commit. @@ -512,7 +512,7 @@ Args: step_name (str): If not None, override the step name. attempts (int): Number of times to try the request before failing. -— **def [download\_file](/recipes/recipe_modules/gitiles/api.py#113)(self, repository_url, file_path, branch='master', step_name=None, attempts=None, \*\*kwargs):** +— **def [download\_file](/recipes/recipe_modules/gitiles/api.py#118)(self, repository_url, file_path, branch='master', step_name=None, attempts=None, \*\*kwargs):** Downloads raw file content from a Gitiles repository. @@ -526,7 +526,7 @@ Args: Returns: Raw file content. -— **def [log](/recipes/recipe_modules/gitiles/api.py#51)(self, url, ref, limit=0, cursor=None, step_name=None, attempts=None, \*\*kwargs):** +— **def [log](/recipes/recipe_modules/gitiles/api.py#56)(self, url, ref, limit=0, cursor=None, step_name=None, attempts=None, \*\*kwargs):** Returns the most recent commits under the given ref with properties. @@ -549,7 +549,7 @@ Returns: Cursor can be used for subsequent calls to log for paging. If None, signals that there are no more commits to fetch. -— **def [refs](/recipes/recipe_modules/gitiles/api.py#39)(self, url, step_name='refs', attempts=None):** +— **def [refs](/recipes/recipe_modules/gitiles/api.py#44)(self, url, step_name='refs', attempts=None):** Returns a list of refs in the remote repository. ### *recipe_modules* / [gsutil](/recipes/recipe_modules/gsutil) @@ -800,9 +800,9 @@ like checkout or compile), and some of these tests have failed. — **def [RunSteps](/recipes/recipe_modules/git_cl/examples/full.py#17)(api):** ### *recipes* / [gitiles:examples/full](/recipes/recipe_modules/gitiles/examples/full.py) -[DEPS](/recipes/recipe_modules/gitiles/examples/full.py#5): [gitiles](#recipe_modules-gitiles), [recipe\_engine/properties][recipe_engine/recipe_modules/properties] +[DEPS](/recipes/recipe_modules/gitiles/examples/full.py#5): [gitiles](#recipe_modules-gitiles), [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/properties][recipe_engine/recipe_modules/properties] -— **def [RunSteps](/recipes/recipe_modules/gitiles/examples/full.py#11)(api):** +— **def [RunSteps](/recipes/recipe_modules/gitiles/examples/full.py#12)(api):** ### *recipes* / [gsutil:examples/full](/recipes/recipe_modules/gsutil/examples/full.py) [DEPS](/recipes/recipe_modules/gsutil/examples/full.py#5): [gsutil](#recipe_modules-gsutil), [recipe\_engine/path][recipe_engine/recipe_modules/path] diff --git a/recipes/recipe_modules/gitiles/api.py b/recipes/recipe_modules/gitiles/api.py index 51cc31749..0f79dedd2 100644 --- a/recipes/recipe_modules/gitiles/api.py +++ b/recipes/recipe_modules/gitiles/api.py @@ -32,6 +32,11 @@ class Gitiles(recipe_api.RecipeApi): args.extend(['--log-limit', log_limit]) if log_start is not None: args.extend(['--log-start', log_start]) + accept_statuses = kwargs.pop('accept_statuses', None) + if accept_statuses: + args.extend([ + '--accept-statuses', + ','.join([str(s) for s in accept_statuses])]) a = self.m.python( step_name, self.resource('gerrit_client.py'), args, **kwargs) return a diff --git a/recipes/recipe_modules/gitiles/examples/full.expected/basic.json b/recipes/recipe_modules/gitiles/examples/full.expected/basic.json index e151c6f63..568ee7643 100644 --- a/recipes/recipe_modules/gitiles/examples/full.expected/basic.json +++ b/recipes/recipe_modules/gitiles/examples/full.expected/basic.json @@ -529,6 +529,24 @@ ], "name": "fetch master:OWNERS" }, + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::gitiles]/resources/gerrit_client.py", + "--json-file", + "/path/to/tmp/json", + "--url", + "https://chromium.googlesource.com/chromium/src/+/master/NONEXISTENT", + "--format", + "text", + "--attempts", + "1", + "--accept-statuses", + "404" + ], + "name": "fetch master:NONEXISTENT" + }, { "name": "$result", "recipe_result": null, diff --git a/recipes/recipe_modules/gitiles/examples/full.py b/recipes/recipe_modules/gitiles/examples/full.py index 71c716d01..9dca3ce20 100644 --- a/recipes/recipe_modules/gitiles/examples/full.py +++ b/recipes/recipe_modules/gitiles/examples/full.py @@ -18,6 +18,8 @@ def RunSteps(api): data = api.gitiles.download_file(url, 'OWNERS', attempts=5) assert data == 'foobar' + data = api.gitiles.download_file(url, 'NONEXISTENT', attempts=1, + accept_statuses=[404]) def GenTests(api): @@ -58,4 +60,8 @@ def GenTests(api): 'fetch master:OWNERS', api.gitiles.make_encoded_file('foobar') ) + + api.step_data( + 'fetch master:NONEXISTENT', + api.gitiles.make_encoded_file('') + ) ) diff --git a/recipes/recipe_modules/gitiles/resources/gerrit_client.py b/recipes/recipe_modules/gitiles/resources/gerrit_client.py index 6de2102af..a34f23ce0 100755 --- a/recipes/recipe_modules/gitiles/resources/gerrit_client.py +++ b/recipes/recipe_modules/gitiles/resources/gerrit_client.py @@ -113,19 +113,22 @@ def main(arguments): 'instead.') query_params['format'] = args.format + kwargs = {} + accept_statuses = frozenset([int(s) for s in args.accept_statuses.split(',')]) + if accept_statuses: + kwargs['accept_statuses'] = accept_statuses + # Choose handler. if args.format == 'json': def handler(conn): - return ReadHttpJsonResponse(conn) + return ReadHttpJsonResponse(conn, **kwargs) elif args.format == 'text': # Text fetching will pack the text into structured JSON. def handler(conn): - result = ReadHttpResponse(conn).read() - if not result: - return None + result = ReadHttpResponse(conn, **kwargs).read() # Wrap in a structured JSON for export to recipe module. return { - 'value': result, + 'value': result or None, } if args.log_start: @@ -183,6 +186,10 @@ def create_argparser(): 'This value can be typically be taken from json result of previous ' 'call to log, which returns next page start commit as "next" key. ' 'Only for https:////+log/... gitiles request.') + parser.add_argument( + '--accept-statuses', type=str, default='200', + help='Comma-separated list of Status codes to accept as "successful" ' + 'HTTP responses.') return parser