Merge lp:~twom/conn-check/port-not-none into lp:conn-check

Proposed by Tom Wardill on 2018-06-04
Status: Merged
Approved by: Tom Wardill on 2018-06-04
Approved revision: 142
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~twom/conn-check/port-not-none
Merge into: lp:conn-check
Diff against target: 70 lines (+32/-6)
2 files modified
conn_check/main.py (+22/-6)
tests.py (+10/-0)
To merge this branch: bzr merge lp:~twom/conn-check/port-not-none
Reviewer Review Type Date Requested Status
Simon Davy 2018-06-04 Approve on 2018-06-04
Review via email: mp+347365@code.launchpad.net

Commit message

Handle missing port in checks, suppress traceback of config errors

To post a comment you must log in.
lp:~twom/conn-check/port-not-none updated on 2018-06-04
139. By Tom Wardill on 2018-06-04

Test the port that we're using

140. By Tom Wardill on 2018-06-04

Merge master

141. By Tom Wardill on 2018-06-04

add default postgres port

142. By Tom Wardill on 2018-06-04

Can't use a default there, it's not postgres specific

Simon Davy (bloodearnest) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'conn_check/main.py'
2--- conn_check/main.py 2018-01-29 14:08:16 +0000
3+++ conn_check/main.py 2018-06-04 16:14:17 +0000
4@@ -1,3 +1,4 @@
5+
6 from __future__ import print_function
7 from __future__ import unicode_literals
8 from future import standard_library
9@@ -73,7 +74,15 @@
10 check_description))
11
12 if 'port' in check_description:
13- check_description['port'] = int(check_description['port'])
14+ port = check_description.get('port')
15+ try:
16+ check_description['port'] = int(port)
17+ except TypeError:
18+ # swap the TypeError for the more generic AssertionError
19+ # that we are using for configuration faults
20+ # and is caught further up
21+ raise AssertionError('Port is invalid for check: {}'.format(
22+ check_description))
23
24 res = check['fn'](**check_description)
25 return res
26@@ -347,11 +356,18 @@
27 def run(self):
28 """Run/validate/dry-run the given command with options."""
29
30- checks = build_checks(self.descriptions,
31- self.options.connect_timeout,
32- self.options.include_tags,
33- self.options.exclude_tags,
34- self.options.dry_run)
35+ try:
36+ checks = build_checks(self.descriptions,
37+ self.options.connect_timeout,
38+ self.options.include_tags,
39+ self.options.exclude_tags,
40+ self.options.dry_run)
41+ except AssertionError as e:
42+ # Configuration fault, suppress traceback
43+ # but display message
44+ self.output.write(str(e))
45+ self.output.flush()
46+ return 4
47
48 if not self.options.validate:
49 if not self.options.dry_run:
50
51=== modified file 'tests.py'
52--- tests.py 2018-01-25 20:58:24 +0000
53+++ tests.py 2018-06-04 16:14:17 +0000
54@@ -300,6 +300,16 @@
55 str(e),
56 "host missing from check: {}".format(description))
57
58+ def test_check_from_description_port_not_set(self):
59+ description = {'type': 'tcp', 'host': 'localhost', 'port': None}
60+ e = self.assertRaises(AssertionError,
61+ check_from_description,
62+ description)
63+ self.assertEqual(
64+ str(e),
65+ "Port is invalid for check: {}".format(description)
66+ )
67+
68 def test_check_from_description_makes_check(self):
69 description = {'type': 'tcp', 'host': 'localhost', 'port': '8080'}
70 result = check_from_description(description)

Subscribers

People subscribed via source and target branches