PR#367: Limit results returned when verbose=true - greenwave

greenwave

#367 Limit results returned when verbose=true

Closed 5 years ago by gnaponie. Opened 5 years ago by gnaponie.

gnaponie/greenwave limit-results-verbose into master

Limit results returned when verbose=true

Giulia Naponiello • 5 years ago

0ddb5ea

functional-tests/test_api_v1.py

file modified

+26

		`@@ -279,6 +279,32 @@`
		`assert res_data['satisfied_requirements'] == expected_satisfied_requirements`


		`+ def test_make_a_decision_with_verbose_flag_too_many_results(requests_session, greenwave_server,`
		`+ testdatabuilder):`
		`+ nvr = testdatabuilder.unique_nvr()`
		`+ results = []`
		`+ for i in range(0, 21):`
		`+ results.append(testdatabuilder.create_result(item=nvr,`
		`+ testcase_name='rhproduct.default.sanity',`
		`+ outcome='PASSED'))`
		`+ data = {`
		`+ 'decision_context': 'bodhi_update_push_stable',`
		`+ 'product_version': 'fedora-26',`
		`+ 'subject_type': 'koji_build',`
		`+ 'subject_identifier': nvr,`
		`+ 'verbose': True,`
		`+ }`
		`+`
		`+ r = requests_session.post(greenwave_server + 'api/v1.0/decision',`
		`+ headers={'Content-Type': 'application/json'},`
		`+ data=json.dumps(data))`
		`+ assert r.status_code == 200`
		`+ res_data = r.json()`
		`+`
		`+ assert len(res_data['results']) == 20`
		`+ assert res_data['results'] == list(reversed(results))[:-1]`
		`+`
		`+`
		`def test_make_a_decision_with_verbose_flag_and_multiple_nvrs_with_results(`
		`requests_session, greenwave_server, testdatabuilder):`
		`build_nvrs = [testdatabuilder.unique_nvr(), testdatabuilder.unique_nvr()]`

greenwave/api_v1.py

file modified

+1 -1

		`@@ -360,7 +360,7 @@`

		`if verbose:`
		`# Retrieve test results for all items when verbose output is requested.`
		`- verbose_results.extend(results_retriever.retrieve(subject_type, subject_identifier))`
		`+ verbose_results.extend(results_retriever.retrieve(subject_type, subject_identifier, limit=20))`
		`verbose_waivers.extend(waivers)`

		`response = {`

greenwave/resources.py

file modified

+15 -11

		`@@ -54,15 +54,15 @@`
		`self.verify = verify`
		`self.url = url`

		`- def retrieve(self, subject_type, subject_identifier, testcase=None):`
		`+ def retrieve(self, subject_type, subject_identifier, testcase=None, limit=None):`
		`"""`
		`Return generator over results.`
		`"""`
		`- for result in self._retrieve_helper(subject_type, subject_identifier, testcase):`
		`+ for result in self._retrieve_helper(subject_type, subject_identifier, testcase, limit):`
		`if result['id'] not in self.ignore_results:`
		`yield result`

		`- def _retrieve_helper(self, subject_type, subject_identifier, testcase):`
		`+ def _retrieve_helper(self, subject_type, subject_identifier, testcase, limit=None):`
		`cache_key = results_cache_key(`
		`subject_type, subject_identifier, testcase)`

		`@@ -75,14 +75,18 @@`

		`while cached_results.can_fetch_more:`
		`cached_results.last_page += 1`
		`- results = self._retrieve_page(`
		`- cached_results.last_page, subject_type, subject_identifier,`
		`- testcase)`
		`- cached_results.results.extend(results)`
		`- cached_results.can_fetch_more = bool(results)`
		`- self.cache.set(cache_key, cached_results)`
		`- for result in results:`
		`- yield result`
		`+ if limit is None or (limit is not None and cached_results.last_page < limit):`
lholecek commented 5 years ago if limit is None or cached_results.last_page < limit
		`+ results = self._retrieve_page(`
		`+ cached_results.last_page, subject_type, subject_identifier,`
		`+ testcase)`
		`+ cached_results.results.extend(results)`
		`+ cached_results.can_fetch_more = bool(results)`
		`+ self.cache.set(cache_key, cached_results)`
		`+ for result in results:`
		`+ yield result`
		`+ else:`
		`+ log.warning("The list of results was truncated. Too many items.")`
		`+ break`

		`def _make_request(self, params):`
		`response = requests_session.get(`

gnaponie commented 5 years ago

When asked for a decision with verbose flag = true, Greenwave returns
the list of matching results for that decision. But when those
results are too many, the request returns with a timeout. Let's just
return the first 20 results and log that the reply is truncated for
debugging reasons.

lholecek commented on line 32 of greenwave/resources.py 5 years ago

if limit is None or cached_results.last_page < limit

gnaponie commented 5 years ago

I think we are going to drop this PR... some discussion on IRC... about the fact that maybe return only the first 20 results it's not a good thing and we might want to increase the limit when asking for the results to resultsdb... That was setted to 1 for a reason, let's see if it's good to reconsider that.

Pull-Request has been closed by gnaponie

5 years ago