PR#100: Refactor get_json - fedora-qa/oraculum

fedora-qa / oraculum

#100 Refactor get_json

Merged 3 years ago by frantisekz. Opened 3 years ago by frantisekz.

refactor_get_json into master

Refactor get_json

František Zatloukal • 3 years ago

744eef5

oraculum/utils/dashboard_helpers.py

file modified

+37 -21

		`@@ -36,10 +36,10 @@`
		`from oraculum import app, CACHE, db`
		`from oraculum.models.dashboard_users import DashboardUserData`

		`-`
		`- def get_json(json_url, attempt_retry=True, log_errors=True, ignore_json_errors=False):`
		`+ def get_resource(url, attempt_retry=True, log_errors=True):`
		`"""`
		`- Returns json data from provided url`
		`+ Returns data from provided url`
		`+ Returns None on error`
		`"""`
		`session = requests.Session()`

		`@@ -51,35 +51,51 @@`
		`else:`
		`session.mount('https://', HTTPAdapter())`
		`try:`
		`- resp = session.get(json_url, timeout=60)`
		`+ resp = session.get(url, timeout=60)`
		`except (ConnectionError, RetryError):`
		`- # Return none if retrying failed too`
		`if log_errors:`
		`- app.logger.error("Request to %s failed." % json_url)`
		`- return None`
		`- if not attempt_retry:`
		`- try:`
		`- return json.loads(resp.text)`
		`- except JSONDecodeError:`
		`- if ignore_json_errors:`
		`- return resp.text`
		`- if log_errors:`
		`- app.logger.error("Request to %s failed because resource returned invalid data." % json_url)`
		`+ app.logger.error("Request to %s failed." % url)`
		`+ if not attempt_retry:`
		`return None`
		`+ else:`
		`+ get_resource(url, attempt_retry=False, log_errors=log_errors)`
		`+`
		`+ return resp.text`
		`+`
		`+`
		`+ def get_json(json_url, attempt_retry=True, log_errors=True):`
		`+ """`
		`+ Returns json data from provided url`
		`+ Returns None on error`
		`+ """`
		`+ json_resp = get_resource(json_url, attempt_retry=attempt_retry, log_errors=log_errors)`
		`try:`
		`- return json.loads(resp.text)`
		`+ return json.loads(json_resp)`
		`except JSONDecodeError:`
		`- if ignore_json_errors:`
		`- return resp.text`
		`- get_json(json_url, attempt_retry=False, log_errors=log_errors, ignore_json_errors=ignore_json_errors)`
		`+ if log_errors:`
		`+ app.logger.error("Request to %s failed because resource returned invalid json data." % json_url)`
		`+ return None`
		`+`
		`+ def get_yaml(yaml_url, attempt_retry=True, log_errors=True):`
		`+ """`
		`+ Returns yaml data from provided url`
		`+ Returns None on error`
		`+ """`
		`+ yaml_resp = get_resource(yaml_url, attempt_retry=attempt_retry, log_errors=log_errors)`
		`+ try:`
		`+ return yaml.safe_load(yaml_resp)`
		`+ except yaml.YAMLError:`
		`+ if log_errors:`
		`+ app.logger.error("Request to %s failed because resource returned invalid yaml data." % yaml_url)`
		`+ return None`

		`def branched_frozen():`
		`"""`
		`Returns true if current branched is frozen`
		`"""`
		`- result = get_json("https://pagure.io/fedora-infra/ansible/raw/master/f/vars/all/Frozen.yaml", ignore_json_errors=True)`
		`+ result = get_yaml("https://pagure.io/fedora-infra/ansible/raw/master/f/vars/all/Frozen.yaml")`
		`try:`
		`- return yaml.safe_load(result)["Frozen"]`
		`+ return result["Frozen"]`
		`except (KeyError, AttributeError):`
		`app.logger.error("Failed to detect freeze status for branched...")`
		`return None`

oraculum/utils/schedule.py

file modified

+2 -2

		`@@ -27,7 +27,7 @@`
		`import pytz`

		`from oraculum import app`
		`- from oraculum.utils.dashboard_helpers import get_json`
		`+ from oraculum.utils.dashboard_helpers import get_resource`

		`def _is_relevant(summary):`
		`relevant = ['Branch Fedora', 'Beta Freeze', 'Beta Release Public Availability', 'Final Freeze', 'Final Release Public Availability']`
		`@@ -45,7 +45,7 @@`
		`releases = CACHE.get("fedora_releases")`
		`release = releases["branched"] or releases["rawhide"]`

		`- resp = get_json(app.config["SCHEDULE_URL"] % (release, release), ignore_json_errors=True)`
		`+ resp = get_resource(app.config["SCHEDULE_URL"] % (release, release))`
		`data = []`

		`if not resp:`

tests/test_packager_dashboard_helpers.py

file modified

+33 -2

		`@@ -88,7 +88,38 @@`

		`def test_branched_frozen(self, monkeypatch):`

		`- yaml_response = "Frozen: True\n"`
		`+ mocked_response = "Frozen: True\n"`

		`- monkeypatch.setattr(dashboard_helpers, 'get_json', mock.MagicMock(return_value=yaml_response))`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=mocked_response))`
		`assert dashboard_helpers.branched_frozen()`
		`+`
		`+ def test_get_json(self, monkeypatch):`
		`+`
		`+ valid_json = '{"fedora":{"branched":null,"branched_frozen":null,"rawhide":34,"stable":33,"values":[32,33,34]}}'`
		`+ invalid_json = '{"fedora:"branched:null,"branched_frozen":nu,"rawhide":?*,"stable":33,"values":[32,33,34]}}'`
		`+ not_a_json = 'Frozen: True\n'`
		`+`
		`+ expected_valid_res = {'fedora': {'branched': None, 'branched_frozen': None, 'rawhide': 34, 'stable': 33, 'values': [32, 33, 34]}}`
		`+`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=valid_json))`
		`+ assert dashboard_helpers.get_json("mocked_url") == expected_valid_res`
		`+`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=invalid_json))`
		`+ assert not dashboard_helpers.get_json("mocked_url")`
		`+`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=not_a_json))`
		`+ assert not dashboard_helpers.get_json("mocked_url")`
		`+`
		`+ def test_get_yaml(self, monkeypatch):`
		`+`
		`+ valid_yaml = 'Frozen: True\n'`
		`+ invalid_yaml = 'Frozen: {{ True\n'`
		`+ # no not_a_yaml here as yaml.safe_load() seems to try and mostly work with almost anything...`
		`+`
		`+ expected_valid_res = {'Frozen': True}`
		`+`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=valid_yaml))`
		`+ assert dashboard_helpers.get_yaml("mocked_url") == expected_valid_res`
		`+`
		`+ monkeypatch.setattr(dashboard_helpers, 'get_resource', mock.MagicMock(return_value=invalid_yaml))`
		`+ assert not dashboard_helpers.get_yaml("mocked_url")`