diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 88bb2e52c..6cdd59d1e 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.2.0 +current_version = 0.2.1 commit = True message = chore: bump covidcast-indicators to {new_version} tag = False diff --git a/_delphi_utils_python/.bumpversion.cfg b/_delphi_utils_python/.bumpversion.cfg index 6344dd87c..29f582b1a 100644 --- a/_delphi_utils_python/.bumpversion.cfg +++ b/_delphi_utils_python/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.2.0 +current_version = 0.2.1 commit = True message = chore: bump delphi_utils to {new_version} tag = False diff --git a/_delphi_utils_python/delphi_utils/__init__.py b/_delphi_utils_python/delphi_utils/__init__.py index 961cc40d1..314cb8296 100644 --- a/_delphi_utils_python/delphi_utils/__init__.py +++ b/_delphi_utils_python/delphi_utils/__init__.py @@ -14,4 +14,4 @@ from .signal import add_prefix from .nancodes import Nans -__version__ = "0.2.0" +__version__ = "0.2.1" diff --git a/_delphi_utils_python/delphi_utils/validator/dynamic.py b/_delphi_utils_python/delphi_utils/validator/dynamic.py index 439fc738e..b81a445d1 100644 --- a/_delphi_utils_python/delphi_utils/validator/dynamic.py +++ b/_delphi_utils_python/delphi_utils/validator/dynamic.py @@ -525,7 +525,7 @@ def outlier_nearby(frame): if source_outliers.shape[0] > 0: for time_val in source_outliers["time_value"].unique(): - report.add_raised_error( + report.add_raised_warning( ValidationFailure( "check_positive_negative_spikes", time_val, @@ -637,7 +637,7 @@ def check_avg_val_vs_reference(self, df_to_test, df_to_reference, checking_date, thres["mean_abs_z"])).any() if mean_z_high or mean_abs_z_high: - report.add_raised_error( + report.add_raised_warning( ValidationFailure( "check_test_vs_reference_avg_changed", checking_date, diff --git a/_delphi_utils_python/delphi_utils/validator/report.py b/_delphi_utils_python/delphi_utils/validator/report.py index aff9196e5..f0840de95 100644 --- a/_delphi_utils_python/delphi_utils/validator/report.py +++ b/_delphi_utils_python/delphi_utils/validator/report.py @@ -103,10 +103,35 @@ def log(self, logger=None): checks_suppressed = self.num_suppressed, warnings = len(self.raised_warnings), phase="validation") + # Threshold for slack alerts if warnings are excessive, + # Currently extremely strict, set by observation of 1 month's logs + excessive_warnings = self.total_checks > 0 and \ + (len(self.raised_warnings) > 200 or \ + len(self.raised_warnings) / self.total_checks > 0.015) + if excessive_warnings: + logger.info("Excessive number of warnings", + data_source = self.data_source, + checks_run = self.total_checks, + checks_failed = len(self.unsuppressed_errors), + checks_suppressed = self.num_suppressed, + warnings = len(self.raised_warnings), + phase = "validation") for error in self.unsuppressed_errors: - logger.critical(str(error), phase="validation") + date_str = "*" if error.date is None else error.date.isoformat() + logger.critical(str(error), + phase = "validation", + error_name = error.check_name, + signal = error.signal, + resolution = error.geo_type, + date = date_str) for warning in self.raised_warnings: - logger.warning(str(warning), phase="validation") + date_str = "*" if warning.date is None else warning.date.isoformat() + logger.warning(str(warning), + phase="validation", + error_name = warning.check_name, + signal = warning.signal, + resolution = warning.geo_type, + date = date_str) def print_and_exit(self, logger=None, die_on_failures=True): """Print results and exit. diff --git a/_delphi_utils_python/setup.py b/_delphi_utils_python/setup.py index bd2e48015..8d361c0de 100644 --- a/_delphi_utils_python/setup.py +++ b/_delphi_utils_python/setup.py @@ -25,7 +25,7 @@ setup( name="delphi_utils", - version="0.2.0", + version="0.2.1", description="Shared Utility Functions for Indicators", long_description=long_description, long_description_content_type="text/markdown", diff --git a/_delphi_utils_python/tests/validator/test_dynamic.py b/_delphi_utils_python/tests/validator/test_dynamic.py index 5c92d7e99..321ce63fb 100644 --- a/_delphi_utils_python/tests/validator/test_dynamic.py +++ b/_delphi_utils_python/tests/validator/test_dynamic.py @@ -202,8 +202,8 @@ def test_10x_val(self): test_df, ref_df, datetime.combine(date.today(), datetime.min.time()), "geo", "signal", report) - assert len(report.raised_errors) == 1 - assert report.raised_errors[0].check_name == "check_test_vs_reference_avg_changed" + assert len(report.raised_warnings) == 1 + assert report.raised_warnings[0].check_name == "check_test_vs_reference_avg_changed" def test_100x_val(self): validator = DynamicValidator(self.params) @@ -223,8 +223,8 @@ def test_100x_val(self): test_df, ref_df, datetime.combine(date.today(), datetime.min.time()), "geo", "signal", report) - assert len(report.raised_errors) == 1 - assert report.raised_errors[0].check_name == "check_test_vs_reference_avg_changed" + assert len(report.raised_warnings) == 1 + assert report.raised_warnings[0].check_name == "check_test_vs_reference_avg_changed" def test_1000x_val(self): validator = DynamicValidator(self.params) @@ -244,8 +244,8 @@ def test_1000x_val(self): test_df, ref_df, datetime.combine(date.today(), datetime.min.time()), "geo", "signal", report) - assert len(report.raised_errors) == 1 - assert report.raised_errors[0].check_name == "check_test_vs_reference_avg_changed" + assert len(report.raised_warnings) == 1 + assert report.raised_warnings[0].check_name == "check_test_vs_reference_avg_changed" class TestDataOutlier: @@ -292,8 +292,8 @@ def test_pos_outlier(self): validator.check_positive_negative_spikes( test_df, ref_df, "state", "signal", report) - assert len(report.raised_errors) == 2 - assert report.raised_errors[0].check_name == "check_positive_negative_spikes" + assert len(report.raised_warnings) == 2 + assert report.raised_warnings[0].check_name == "check_positive_negative_spikes" def test_neg_outlier(self): validator = DynamicValidator(self.params) @@ -329,8 +329,8 @@ def test_neg_outlier(self): validator.check_positive_negative_spikes( test_df, ref_df, "state", "signal", report) - assert len(report.raised_errors) == 2 - assert report.raised_errors[0].check_name == "check_positive_negative_spikes" + assert len(report.raised_warnings) == 2 + assert report.raised_warnings[0].check_name == "check_positive_negative_spikes" def test_zero_outlier(self): validator = DynamicValidator(self.params) @@ -365,8 +365,8 @@ def test_zero_outlier(self): validator.check_positive_negative_spikes( test_df, ref_df, "state", "signal", report) - assert len(report.raised_errors) == 1 - assert report.raised_errors[0].check_name == "check_positive_negative_spikes" + assert len(report.raised_warnings) == 1 + assert report.raised_warnings[0].check_name == "check_positive_negative_spikes" def test_no_outlier(self): validator = DynamicValidator(self.params) @@ -402,7 +402,7 @@ def test_no_outlier(self): validator.check_positive_negative_spikes( test_df, ref_df, "state", "signal", report) - assert len(report.raised_errors) == 0 + assert len(report.raised_warnings) == 0 def test_source_api_overlap(self): validator = DynamicValidator(self.params) @@ -438,5 +438,5 @@ def test_source_api_overlap(self): validator.check_positive_negative_spikes( test_df, ref_df, "state", "signal", report) - assert len(report.raised_errors) == 2 - assert report.raised_errors[0].check_name == "check_positive_negative_spikes" + assert len(report.raised_warnings) == 2 + assert report.raised_warnings[0].check_name == "check_positive_negative_spikes" diff --git a/_delphi_utils_python/tests/validator/test_report.py b/_delphi_utils_python/tests/validator/test_report.py index 2478476e2..a904b7da3 100644 --- a/_delphi_utils_python/tests/validator/test_report.py +++ b/_delphi_utils_python/tests/validator/test_report.py @@ -12,6 +12,8 @@ class TestValidationReport: ERROR_2 = ValidationFailure("bad", filename="20201107_county_sig2.csv", message="msg 2") + WARNING_1 = ValidationFailure("wrong import", date = None) + WARNING_2 = ValidationFailure("right import", date = None) def test_add_raised_unsuppressed_error(self): """Test that an unsupressed error shows up in the unsuppressed error list.""" @@ -36,8 +38,8 @@ def test_str(self): report.increment_total_checks() report.increment_total_checks() report.increment_total_checks() - report.add_raised_warning(ImportWarning("wrong import")) - report.add_raised_warning(ImportWarning("right import")) + report.add_raised_warning(self.WARNING_1) + report.add_raised_warning(self.WARNING_2) report.add_raised_error(self.ERROR_1) report.add_raised_error(self.ERROR_2) @@ -48,15 +50,27 @@ def test_log(self): report.increment_total_checks() report.increment_total_checks() report.increment_total_checks() - report.add_raised_warning(ImportWarning("wrong import")) - report.add_raised_warning(ImportWarning("right import")) + report.add_raised_warning(self.WARNING_1) + report.add_raised_warning(self.WARNING_2) report.add_raised_error(self.ERROR_1) report.add_raised_error(self.ERROR_2) report.log(mock_logger) mock_logger.critical.assert_called_once_with( "bad failed for sig2 at resolution county on 2020-11-07: msg 2", - phase = "validation") + phase = "validation", error_name = "bad", + signal = "sig2", resolution = "county", + date = "2020-11-07") mock_logger.warning.assert_has_calls( - [mock.call("wrong import",phase = "validation"), - mock.call("right import", phase = "validation")]) + [mock.call("wrong import failed for None at resolution None on *: ", + phase = "validation", + error_name = "wrong import", + signal = None, + resolution = None, + date = '*'), + mock.call("right import failed for None at resolution None on *: ", + phase = "validation", + error_name = "right import", + signal = None, + resolution = None, + date = '*')]) diff --git a/ansible/templates/quidel_covidtest-params-prod.json.j2 b/ansible/templates/quidel_covidtest-params-prod.json.j2 index 8d56d4166..6047b0469 100644 --- a/ansible/templates/quidel_covidtest-params-prod.json.j2 +++ b/ansible/templates/quidel_covidtest-params-prod.json.j2 @@ -26,7 +26,17 @@ "min_expected_lag": {"all": "5"}, "max_expected_lag": {"all": "5"}, "dry_run": true, - "suppressed_errors": [] + "suppressed_errors": [ + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "hrr"}, + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "msa"}, + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "county"} + ] }, "static": { "minimum_sample_size": 50 diff --git a/quidel_covidtest/params.json.template b/quidel_covidtest/params.json.template index 60174b232..49649c9ff 100644 --- a/quidel_covidtest/params.json.template +++ b/quidel_covidtest/params.json.template @@ -27,7 +27,17 @@ "min_expected_lag": {"all": "5"}, "max_expected_lag": {"all": "5"}, "dry_run": true, - "suppressed_errors": [] + "suppressed_errors": [ + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "hrr"}, + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "msa"}, + {"check_name": "check_rapid_change_num_rows", + "signal": "covid_ag_raw_pct_positive", + "geo_type": "county"} + ] }, "static": { "minimum_sample_size": 50