From f3986e5a6f382af5ac40cc707b63721b8297dbb5 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Thu, 5 Sep 2019 11:02:08 -0400 Subject: [PATCH] Issue #419 - refactor Runner.print_issue() to utils.issue_string_tuple() --- awslimitchecker/runner.py | 47 +----- awslimitchecker/tests/test_runner.py | 218 +++++---------------------- awslimitchecker/tests/test_utils.py | 164 +++++++++++++++++++- awslimitchecker/utils.py | 45 ++++++ 4 files changed, 253 insertions(+), 221 deletions(-) diff --git a/awslimitchecker/runner.py b/awslimitchecker/runner.py index 4b0c1208..cf705a0a 100644 --- a/awslimitchecker/runner.py +++ b/awslimitchecker/runner.py @@ -45,7 +45,7 @@ import time from .checker import AwsLimitChecker -from .utils import StoreKeyValuePair, dict2cols, color_output +from .utils import StoreKeyValuePair, dict2cols, issue_string_tuple from .limit import SOURCE_TA, SOURCE_API from .metrics import MetricsProvider from .alerts import AlertProvider @@ -312,47 +312,6 @@ def show_usage(self): v=limits[svc][lim].get_current_usage_str()) print(dict2cols(data)) - def print_issue(self, service_name, limit, crits, warns): - """ - Return a 2-tuple of key (service/limit name)/value (usage) strings - describing a limit that has crossed its threshold. - - :param service_name: the name of the service - :type service_name: str - :param limit: the Limit this relates to - :type limit: :py:class:`~.AwsLimit` - :param crits: the specific usage values that crossed the critical - threshold - :type usage: :py:obj:`list` of :py:class:`~.AwsLimitUsage` - :param crits: the specific usage values that crossed the warning - threshold - :type usage: :py:obj:`list` of :py:class:`~.AwsLimitUsage` - :returns: 2-tuple of strings describing crossed thresholds, - first describing the service and limit name and second listing the - limit and usage - :rtype: tuple - """ - usage_str = '' - if len(crits) > 0: - tmp = 'CRITICAL: ' - tmp += ', '.join([str(x) for x in sorted(crits)]) - usage_str += color_output(tmp, 'red', colorize=self.colorize) - if len(warns) > 0: - if len(crits) > 0: - usage_str += ' ' - tmp = 'WARNING: ' - tmp += ', '.join([str(x) for x in sorted(warns)]) - usage_str += color_output(tmp, 'yellow', colorize=self.colorize) - k = "{s}/{l}".format( - s=service_name, - l=limit.name, - ) - v = "(limit {v}) {u}".format( - v=limit.get_limit(), - u=usage_str, - ) - return k, v - def check_thresholds(self, metrics=None): have_warn = False have_crit = False @@ -382,7 +341,9 @@ def check_thresholds(self, metrics=None): have_crit = True if len(warns) > 0: have_warn = True - k, v = self.print_issue(svc, limit, crits, warns) + k, v = issue_string_tuple( + svc, limit, crits, warns, colorize=self.colorize + ) columns[k] = v d2c = dict2cols(columns) print(d2c) diff --git a/awslimitchecker/tests/test_runner.py b/awslimitchecker/tests/test_runner.py index 7376f078..7e282de6 100644 --- a/awslimitchecker/tests/test_runner.py +++ b/awslimitchecker/tests/test_runner.py @@ -869,11 +869,12 @@ def test_many_problems(self): } mock_checker.get_limits.return_value = {} - def se_print(cls, s, l, c, w): + def se_print(s, l, c, w, colorize=True): return ('{s}/{l}'.format(s=s, l=l.name), '') self.cls.checker = mock_checker - with patch('awslimitchecker.runner.Runner.print_issue', + self.cls.colorize = False + with patch('%s.issue_string_tuple' % pb, autospec=True) as mock_print: mock_print.side_effect = se_print with patch('awslimitchecker.runner.dict2cols') as mock_d2c: @@ -883,10 +884,19 @@ def se_print(cls, s, l, c, w): call.check_thresholds(use_ta=True, service=None) ] assert mock_print.mock_calls == [ - call(self.cls, 'svc1', mock_limit1, [mock_c1], [mock_w1]), - call(self.cls, 'svc1', mock_limit2, [], [mock_w2]), - call(self.cls, 'svc2', mock_limit3, [], [mock_w3]), - call(self.cls, 'svc2', mock_limit4, [mock_c2], []), + call( + 'svc1', mock_limit1, [mock_c1], [mock_w1], + colorize=False + ), + call( + 'svc1', mock_limit2, [], [mock_w2], colorize=False + ), + call( + 'svc2', mock_limit3, [], [mock_w3], colorize=False + ), + call( + 'svc2', mock_limit4, [mock_c2], [], colorize=False + ), ] assert mock_d2c.mock_calls == [ call({ @@ -931,12 +941,12 @@ def test_when_skip_check(self): } mock_checker.get_limits.return_value = {} - def se_print(cls, s, l, c, w): + def se_print(s, l, c, w, colorize=True): return ('{s}/{l}'.format(s=s, l=l.name), '') self.cls.checker = mock_checker self.cls.skip_check = ['svc1/limit1'] - with patch('awslimitchecker.runner.Runner.print_issue', + with patch('%s.issue_string_tuple' % pb, autospec=True) as mock_print: mock_print.side_effect = se_print with patch('awslimitchecker.runner.dict2cols') as mock_d2c: @@ -947,7 +957,9 @@ def se_print(cls, s, l, c, w): call.check_thresholds(use_ta=True, service=None) ] assert mock_print.mock_calls == [ - call(self.cls, 'svc1', mock_limit2, [], [mock_w2]), + call( + 'svc1', mock_limit2, [], [mock_w2], colorize=True + ), ] assert mock_d2c.mock_calls == [ call({ @@ -986,7 +998,7 @@ def test_warn(self): mock_checker.get_limits.return_value = {} self.cls.checker = mock_checker - with patch('awslimitchecker.runner.Runner.print_issue', + with patch('%s.issue_string_tuple' % pb, autospec=True) as mock_print: mock_print.return_value = ('', '') with patch('awslimitchecker.runner.dict2cols') as mock_d2c: @@ -996,8 +1008,13 @@ def test_warn(self): call.check_thresholds(use_ta=True, service=None) ] assert mock_print.mock_calls == [ - call(self.cls, 'svc1', mock_limit1, [], [mock_w1, mock_w2]), - call(self.cls, 'svc2', mock_limit2, [], [mock_w3]), + call( + 'svc1', mock_limit1, [], [mock_w1, mock_w2], + colorize=True + ), + call( + 'svc2', mock_limit2, [], [mock_w3], colorize=True + ), ] assert res == (1, { 'svc2': { @@ -1031,7 +1048,7 @@ def test_warn_one_service(self): self.cls.checker = mock_checker self.cls.service_name = ['svc2'] - with patch('awslimitchecker.runner.Runner.print_issue', + with patch('%s.issue_string_tuple' % pb, autospec=True) as mock_print: mock_print.return_value = ('', '') with patch('awslimitchecker.runner.dict2cols') as mock_d2c: @@ -1041,7 +1058,9 @@ def test_warn_one_service(self): call.check_thresholds(use_ta=True, service=['svc2']) ] assert mock_print.mock_calls == [ - call(self.cls, 'svc2', mock_limit2, [], [mock_w3]), + call( + 'svc2', mock_limit2, [], [mock_w3], colorize=True + ), ] assert res == (1, { 'svc2': { @@ -1067,7 +1086,7 @@ def test_crit(self): self.cls.checker = mock_checker self.cls.skip_ta = True - with patch('awslimitchecker.runner.Runner.print_issue', + with patch('%s.issue_string_tuple' % pb, autospec=True) as mock_print: mock_print.return_value = ('', '') with patch('awslimitchecker.runner.dict2cols') as mock_d2c: @@ -1077,7 +1096,10 @@ def test_crit(self): call.check_thresholds(use_ta=False, service=None) ] assert mock_print.mock_calls == [ - call(self.cls, 'svc1', mock_limit1, [mock_c1, mock_c2], []), + call( + 'svc1', mock_limit1, [mock_c1, mock_c2], [], + colorize=True + ), ] assert res == (2, { 'svc1': { @@ -1086,166 +1108,6 @@ def test_crit(self): }, ' \n') -class TestPrintIssue(RunnerTester): - - def test_crit_one(self): - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 12 - - c1 = AwsLimitUsage(mock_limit, 56) - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [c1], - [] - ) - assert res == ('svcname/limitname', - '(limit 12) xXCRITICAL: 56Xx') - assert m_co.mock_calls == [ - call('CRITICAL: 56', 'red', colorize=True) - ] - - def test_crit_multi(self): - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 5 - - c1 = AwsLimitUsage(mock_limit, 10) - c2 = AwsLimitUsage(mock_limit, 12, resource_id='c2id') - c3 = AwsLimitUsage(mock_limit, 8) - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [c1, c2, c3], - [] - ) - assert res == ('svcname/limitname', - '(limit 5) xXCRITICAL: 8, 10, c2id=12Xx') - assert m_co.mock_calls == [ - call('CRITICAL: 8, 10, c2id=12', 'red', colorize=True) - ] - - def test_warn_one(self): - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 12 - - w1 = AwsLimitUsage(mock_limit, 11) - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [], - [w1] - ) - assert res == ('svcname/limitname', '(limit 12) xXWARNING: 11Xx') - assert m_co.mock_calls == [ - call('WARNING: 11', 'yellow', colorize=True) - ] - - def test_warn_multi(self): - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 12 - - w1 = AwsLimitUsage(mock_limit, 11) - w2 = AwsLimitUsage(mock_limit, 10, resource_id='w2id') - w3 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [], - [w1, w2, w3] - ) - assert res == ('svcname/limitname', - '(limit 12) xXWARNING: w2id=10, w3id=10, 11Xx') - assert m_co.mock_calls == [ - call('WARNING: w2id=10, w3id=10, 11', 'yellow', colorize=True) - ] - - def test_both_one(self): - self.cls.colorize = False - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 12 - - c1 = AwsLimitUsage(mock_limit, 10) - w1 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [c1], - [w1] - ) - assert res == ('svcname/limitname', - '(limit 12) xXCRITICAL: 10Xx xXWARNING: w3id=10Xx') - assert m_co.mock_calls == [ - call('CRITICAL: 10', 'red', colorize=False), - call('WARNING: w3id=10', 'yellow', colorize=False) - ] - - def test_both_multi(self): - mock_limit = Mock(spec_set=AwsLimit) - type(mock_limit).name = 'limitname' - mock_limit.get_limit.return_value = 12 - - c1 = AwsLimitUsage(mock_limit, 10) - c2 = AwsLimitUsage(mock_limit, 12, resource_id='c2id') - c3 = AwsLimitUsage(mock_limit, 8) - w1 = AwsLimitUsage(mock_limit, 11) - w2 = AwsLimitUsage(mock_limit, 10, resource_id='w2id') - w3 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') - - def se_color(s, c, colorize=True): - return 'xX%sXx' % s - - with patch('%s.color_output' % pb) as m_co: - m_co.side_effect = se_color - res = self.cls.print_issue( - 'svcname', - mock_limit, - [c1, c2, c3], - [w1, w2, w3] - ) - assert res == ('svcname/limitname', - '(limit 12) xXCRITICAL: 8, 10, c2id=12Xx ' - 'xXWARNING: w2id=10, w3id=10, 11Xx') - assert m_co.mock_calls == [ - call('CRITICAL: 8, 10, c2id=12', 'red', colorize=True), - call('WARNING: w2id=10, w3id=10, 11', 'yellow', colorize=True) - ] - - class TestConsoleEntryPoint(RunnerTester): def test_version(self, capsys): @@ -2059,7 +1921,9 @@ def test_no_color(self): with patch('%s.Runner.check_thresholds' % pb, autospec=True) as mock_ct: mock_ct.return_value = 0, {}, 'foo' - with patch('%s.color_output' % pb) as m_co: + with patch( + 'awslimitchecker.utils.color_output' + ) as m_co: m_co.return_value = 'COLORIZED' with pytest.raises(SystemExit): self.cls.console_entry_point() diff --git a/awslimitchecker/tests/test_utils.py b/awslimitchecker/tests/test_utils.py index feb98562..151dc144 100644 --- a/awslimitchecker/tests/test_utils.py +++ b/awslimitchecker/tests/test_utils.py @@ -42,9 +42,11 @@ import sys import termcolor +from awslimitchecker.limit import AwsLimit, AwsLimitUsage from awslimitchecker.utils import ( StoreKeyValuePair, dict2cols, paginate_dict, _get_dict_value_by_path, - _set_dict_value_by_path, _get_latest_version, color_output + _set_dict_value_by_path, _get_latest_version, color_output, + issue_string_tuple ) # https://code.google.com/p/mock/issues/detail?id=249 @@ -475,3 +477,163 @@ def test_not_colored(self): assert color_output( 'foo', 'yellow', colorize=False ) == 'foo' + + +class TestIssueStringTuple(object): + + def test_crit_one(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 12 + + c1 = AwsLimitUsage(mock_limit, 56) + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [c1], + [] + ) + assert res == ('svcname/limitname', + '(limit 12) xXCRITICAL: 56Xx') + assert m_co.mock_calls == [ + call('CRITICAL: 56', 'red', colorize=True) + ] + + def test_crit_multi(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 5 + + c1 = AwsLimitUsage(mock_limit, 10) + c2 = AwsLimitUsage(mock_limit, 12, resource_id='c2id') + c3 = AwsLimitUsage(mock_limit, 8) + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [c1, c2, c3], + [] + ) + assert res == ('svcname/limitname', + '(limit 5) xXCRITICAL: 8, 10, c2id=12Xx') + assert m_co.mock_calls == [ + call('CRITICAL: 8, 10, c2id=12', 'red', colorize=True) + ] + + def test_warn_one(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 12 + + w1 = AwsLimitUsage(mock_limit, 11) + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [], + [w1] + ) + assert res == ('svcname/limitname', '(limit 12) xXWARNING: 11Xx') + assert m_co.mock_calls == [ + call('WARNING: 11', 'yellow', colorize=True) + ] + + def test_warn_multi(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 12 + + w1 = AwsLimitUsage(mock_limit, 11) + w2 = AwsLimitUsage(mock_limit, 10, resource_id='w2id') + w3 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [], + [w1, w2, w3] + ) + assert res == ('svcname/limitname', + '(limit 12) xXWARNING: w2id=10, w3id=10, 11Xx') + assert m_co.mock_calls == [ + call('WARNING: w2id=10, w3id=10, 11', 'yellow', colorize=True) + ] + + def test_both_one(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 12 + + c1 = AwsLimitUsage(mock_limit, 10) + w1 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [c1], + [w1], + colorize=False + ) + assert res == ('svcname/limitname', + '(limit 12) xXCRITICAL: 10Xx xXWARNING: w3id=10Xx') + assert m_co.mock_calls == [ + call('CRITICAL: 10', 'red', colorize=False), + call('WARNING: w3id=10', 'yellow', colorize=False) + ] + + def test_both_multi(self): + mock_limit = Mock(spec_set=AwsLimit) + type(mock_limit).name = 'limitname' + mock_limit.get_limit.return_value = 12 + + c1 = AwsLimitUsage(mock_limit, 10) + c2 = AwsLimitUsage(mock_limit, 12, resource_id='c2id') + c3 = AwsLimitUsage(mock_limit, 8) + w1 = AwsLimitUsage(mock_limit, 11) + w2 = AwsLimitUsage(mock_limit, 10, resource_id='w2id') + w3 = AwsLimitUsage(mock_limit, 10, resource_id='w3id') + + def se_color(s, c, colorize=True): + return 'xX%sXx' % s + + with patch('%s.color_output' % pbm) as m_co: + m_co.side_effect = se_color + res = issue_string_tuple( + 'svcname', + mock_limit, + [c1, c2, c3], + [w1, w2, w3] + ) + assert res == ('svcname/limitname', + '(limit 12) xXCRITICAL: 8, 10, c2id=12Xx ' + 'xXWARNING: w2id=10, w3id=10, 11Xx') + assert m_co.mock_calls == [ + call('CRITICAL: 8, 10, c2id=12', 'red', colorize=True), + call('WARNING: w2id=10, w3id=10, 11', 'yellow', colorize=True) + ] diff --git a/awslimitchecker/utils.py b/awslimitchecker/utils.py index b58d62b2..9df86b8e 100644 --- a/awslimitchecker/utils.py +++ b/awslimitchecker/utils.py @@ -253,3 +253,48 @@ def color_output(s, color, colorize=True): if not colorize: return s return termcolor.colored(s, color) + + +def issue_string_tuple(service_name, limit, crits, warns, colorize=True): + """ + Return a 2-tuple of key (service/limit name)/value (usage) strings + describing a limit that has crossed its threshold. + + :param service_name: the name of the service + :type service_name: str + :param limit: the Limit this relates to + :type limit: :py:class:`~.AwsLimit` + :param crits: the specific usage values that crossed the critical + threshold + :type usage: :py:obj:`list` of :py:class:`~.AwsLimitUsage` + :param crits: the specific usage values that crossed the warning + threshold + :type usage: :py:obj:`list` of :py:class:`~.AwsLimitUsage` + :param colorize: whether or not to colorize output; passed through to + :py:func:`~.color_output`. + :type colorize: bool + :returns: 2-tuple of strings describing crossed thresholds, + first describing the service and limit name and second listing the + limit and usage + :rtype: tuple + """ + usage_str = '' + if len(crits) > 0: + tmp = 'CRITICAL: ' + tmp += ', '.join([str(x) for x in sorted(crits)]) + usage_str += color_output(tmp, 'red', colorize=colorize) + if len(warns) > 0: + if len(crits) > 0: + usage_str += ' ' + tmp = 'WARNING: ' + tmp += ', '.join([str(x) for x in sorted(warns)]) + usage_str += color_output(tmp, 'yellow', colorize=colorize) + k = "{s}/{l}".format( + s=service_name, + l=limit.name, + ) + v = "(limit {v}) {u}".format( + v=limit.get_limit(), + u=usage_str, + ) + return k, v