From 4ad226746db441fb76f4fb2f687749914f0cb81d Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Fri, 4 Oct 2019 16:29:52 +0200 Subject: [PATCH] Don't stop contacting sources after receiving a 401 (Unauthorized) or 403 (Forbidden). Fixes #652. --- .../src/metric_collectors/metric_collector.py | 2 -- .../src/source_collectors/source_collector.py | 11 +---------- .../metric_collectors_tests/test_metrics_collector.py | 4 +--- docs/CHANGELOG.md | 2 +- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/components/collector/src/metric_collectors/metric_collector.py b/components/collector/src/metric_collectors/metric_collector.py index 347fa2e7d9..c44620ce67 100644 --- a/components/collector/src/metric_collectors/metric_collector.py +++ b/components/collector/src/metric_collectors/metric_collector.py @@ -32,8 +32,6 @@ def can_collect(self) -> bool: def next_collection(self) -> datetime: """Return when the metric can/should be collected again.""" - if any([collector.has_invalid_credentials() for collector in self.collectors.values()]): - return datetime.max # Don't try to collect this metric again, except when its configuration changes return min([collector.next_collection() for collector in self.collectors.values()], default=datetime.min) def get(self) -> Measurement: diff --git a/components/collector/src/source_collectors/source_collector.py b/components/collector/src/source_collectors/source_collector.py index 133e492f0f..382dcf64d7 100644 --- a/components/collector/src/source_collectors/source_collector.py +++ b/components/collector/src/source_collectors/source_collector.py @@ -5,7 +5,6 @@ import urllib from abc import ABC, abstractmethod from datetime import datetime, timedelta, timezone -from http import HTTPStatus from typing import cast, Dict, List, Optional, Set, Tuple, Type, Union import requests @@ -26,7 +25,6 @@ class SourceCollector(ABC): def __init__(self, source, datamodel) -> None: self._datamodel = datamodel self.__parameters: Dict[str, Union[str, List[str]]] = source.get("parameters", {}) - self.__has_invalid_credentials = False def __init_subclass__(cls) -> None: SourceCollector.subclasses.add(cls) @@ -89,7 +87,6 @@ def __safely_get_source_responses(self, api_url: URL) -> Tuple[Responses, ErrorM try: responses = self._get_source_responses(api_url) for response in responses: - self.__has_invalid_credentials = response.status_code in (HTTPStatus.UNAUTHORIZED, HTTPStatus.FORBIDDEN) response.raise_for_status() except Exception: # pylint: disable=broad-except error = stable_traceback(traceback.format_exc()) @@ -147,19 +144,13 @@ def next_collection(self) -> datetime: # pylint: disable=no-self-use """Return when this source should be connected again for measurement data.""" return datetime.now() + timedelta(seconds=15 * 60) - def has_invalid_credentials(self) -> bool: - """Return whether this collector has been given invalid credentials.""" - return self.__has_invalid_credentials - class LocalSourceCollector(SourceCollector, ABC): # pylint: disable=abstract-method """Base class for source collectors that do not need to access the network but return static or user-supplied data.""" def _get_source_responses(self, api_url: URL) -> Responses: - fake_response = requests.Response() # Return a fake response so that the parse methods will be called - fake_response.status_code = HTTPStatus.OK - return [fake_response] + return [requests.Response()] # Return a fake response so that the parse methods will be called class UnmergedBranchesSourceCollector(SourceCollector, ABC): # pylint: disable=abstract-method diff --git a/components/collector/tests/unittests/metric_collectors_tests/test_metrics_collector.py b/components/collector/tests/unittests/metric_collectors_tests/test_metrics_collector.py index 4bdc4d807d..8c61eff2ad 100644 --- a/components/collector/tests/unittests/metric_collectors_tests/test_metrics_collector.py +++ b/components/collector/tests/unittests/metric_collectors_tests/test_metrics_collector.py @@ -150,10 +150,8 @@ def test_fetch_twice_with_invalid_credentials(self): self.metrics_response.json.return_value = dict( metric_uuid=dict(report_uuid="report_uuid", addition="sum", type="metric", sources=dict(source_id=dict(type="source", parameters=dict(url="https://url"))))) - unauthorized = Mock() - unauthorized.status_code = 401 side_effect = [ - self.datamodel_response, self.metrics_response, unauthorized, self.datamodel_response, + self.datamodel_response, self.metrics_response, Mock(), self.datamodel_response, self.metrics_response] with patch("requests.get", side_effect=side_effect): with patch("requests.post") as post: diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6f97b76608..0b3a8b0bc3 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Allow for specifying which test results (skipped, failed, errored, and/or passed) to count when using SonarQube as source for the "Tests" metric. Fixes [#634](https://github.com/ICTU/quality-time/issues/634). - Store measurement values for each scale that a metric supports so that the graphs show correct information when the user changes the metric scale. Fixes [#637](https://github.com/ICTU/quality-time/issues/637). -- TODO. Fixes [#652](https://github.com/ICTU/quality-time/issues/652). +- Don't stop contacting sources after receiving a 401 (Unauthorized) or 403 (Forbidden). Fixes [#652](https://github.com/ICTU/quality-time/issues/652). ### Added