curtin

Merge lp:~raharper/curtin/trunk.fix-missing-install-log into lp:~curtin-dev/curtin/trunk

  1. trunk.fix-missing-install-log
  2. Merge into trunk
Proposed by Ryan Harper
Status: Merged
Merged at revision: 392
Proposed branch: lp:~raharper/curtin/trunk.fix-missing-install-log
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 96 lines (+27/-15)
2 files modified
curtin/reporter/handlers.py (+4/-13)
tests/unittests/test_reporter.py (+23/-2)
To merge this branch: bzr merge lp:~raharper/curtin/trunk.fix-missing-install-log
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
curtin developers Pending
Review via email: mp+297340@code.launchpad.net

Commit message

reporting: default webhook handler level to DEBUG, no filtering

The webhook handler in curtin reporting included a default level
of INFO and included a event filter which would not report events
that were of lower importance than what was set. The result was
for unconfigured WebHook handlers, some of the expected messages
including post_files used by maas to collect the curtin install
log were no longer sent.

This patch sets WebHookHandler level to DEBUG, the same level set for
LogHandler and PrintHandler, further we remove the event filtering.
The events themselves include the level value and the event collector
can apply their own filtering as needed. Along with the changes we
clean up some comments and include a new unittest which specifically
tests that a FinishReportingEvent with post_files is posted by the
WebHookHandler.

To post a comment you must log in.
lp:~raharper/curtin/trunk.fix-missing-install-log updated
391. By Ryan Harper

merge from trunk

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:390
https://server-team-jenkins.canonical.com/job/curtin-ci/256/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/253/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/256/rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:391
https://server-team-jenkins.canonical.com/job/curtin-ci/257/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/254/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/257/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I think this is fine. there is some cleanup to do there, where we'd like to avoid passing strings around explicitly (rather than level.DEBUG), but this is fine and will un-break some things.

feel free to do more if you want.

Revision history for this message
Ryan Harper (raharper) wrote :

vmtest passed on diglett, merging to trunk.

----------------------------------------------------------------------
Ran 610 tests in 9712.856s

OK
Tue, 14 Jun 2016 17:15:22 -0500: vmtest end [0] in 9713s

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'curtin/reporter/handlers.py'
--- curtin/reporter/handlers.py 2016-04-04 19:09:47 +0000
+++ curtin/reporter/handlers.py 2016-06-14 14:07:09 +0000
@@ -23,7 +23,7 @@
2323
2424
25class LogHandler(ReportingHandler):25class LogHandler(ReportingHandler):
26 """Publishes events to the cloud-init log at the ``INFO`` log level."""26 """Publishes events to the curtin log at the ``DEBUG`` log level."""
2727
28 def __init__(self, level="DEBUG"):28 def __init__(self, level="DEBUG"):
29 super(LogHandler, self).__init__()29 super(LogHandler, self).__init__()
@@ -39,9 +39,9 @@
39 self.level = level39 self.level = level
4040
41 def publish_event(self, event):41 def publish_event(self, event):
42 """Publish an event to the ``INFO`` log level."""42 """Publish an event to the ``DEBUG`` log level."""
43 logger = logging.getLogger(43 logger = logging.getLogger(
44 '.'.join(['cloudinit', 'reporting', event.event_type, event.name]))44 '.'.join(['curtin', 'reporting', event.event_type, event.name]))
45 logger.log(self.level, event.as_string())45 logger.log(self.level, event.as_string())
4646
4747
@@ -55,7 +55,7 @@
55class WebHookHandler(ReportingHandler):55class WebHookHandler(ReportingHandler):
56 def __init__(self, endpoint, consumer_key=None, token_key=None,56 def __init__(self, endpoint, consumer_key=None, token_key=None,
57 token_secret=None, consumer_secret=None, timeout=None,57 token_secret=None, consumer_secret=None, timeout=None,
58 retries=None, level="INFO"):58 retries=None, level="DEBUG"):
59 super(WebHookHandler, self).__init__()59 super(WebHookHandler, self).__init__()
6060
61 self.oauth_helper = url_helper.OauthUrlHelper(61 self.oauth_helper = url_helper.OauthUrlHelper(
@@ -72,15 +72,6 @@
72 self.headers = {'Content-Type': 'application/json'}72 self.headers = {'Content-Type': 'application/json'}
7373
74 def publish_event(self, event):74 def publish_event(self, event):
75 if isinstance(event.level, int):
76 ev_level = event.level
77 else:
78 try:
79 ev_level = getattr(logging, event.level.upper())
80 except:
81 ev_level = logging.INFO
82 if ev_level < self.level:
83 return
84 try:75 try:
85 return self.oauth_helper.geturl(76 return self.oauth_helper.geturl(
86 url=self.endpoint, data=event.as_dict(),77 url=self.endpoint, data=event.as_dict(),
8778
=== modified file 'tests/unittests/test_reporter.py'
--- tests/unittests/test_reporter.py 2016-04-04 19:09:47 +0000
+++ tests/unittests/test_reporter.py 2016-06-14 14:07:09 +0000
@@ -128,7 +128,8 @@
128 def test_webhook_handler(self, mock_url_helper):128 def test_webhook_handler(self, mock_url_helper):
129 event = events.ReportingEvent(events.START_EVENT_TYPE, 'test_event',129 event = events.ReportingEvent(events.START_EVENT_TYPE, 'test_event',
130 'test event', level='INFO')130 'test event', level='INFO')
131 webhook_handler = handlers.WebHookHandler('127.0.0.1:8000')131 webhook_handler = handlers.WebHookHandler('127.0.0.1:8000',
132 level='INFO')
132 webhook_handler.publish_event(event)133 webhook_handler.publish_event(event)
133 webhook_handler.oauth_helper.geturl.assert_called_with(134 webhook_handler.oauth_helper.geturl.assert_called_with(
134 url='127.0.0.1:8000', data=event.as_dict(),135 url='127.0.0.1:8000', data=event.as_dict(),
@@ -136,7 +137,6 @@
136 event.level = 'DEBUG'137 event.level = 'DEBUG'
137 webhook_handler.oauth_helper.geturl.called = False138 webhook_handler.oauth_helper.geturl.called = False
138 webhook_handler.publish_event(event)139 webhook_handler.publish_event(event)
139 self.assertFalse(webhook_handler.oauth_helper.geturl.called)
140 webhook_handler = handlers.WebHookHandler('127.0.0.1:8000',140 webhook_handler = handlers.WebHookHandler('127.0.0.1:8000',
141 level="INVALID")141 level="INVALID")
142 self.assertEquals(webhook_handler.level, 30)142 self.assertEquals(webhook_handler.level, 30)
@@ -194,3 +194,24 @@
194 base64.b64encode(test_data).decode())194 base64.b64encode(test_data).decode())
195 finally:195 finally:
196 os.remove(tmp[1])196 os.remove(tmp[1])
197
198 @patch('curtin.url_helper.OauthUrlHelper')
199 def test_webhook_handler_post_files(self, mock_url_helper):
200 test_data = b'abcdefg'
201 tmp = tempfile.mkstemp()
202 tmpfname = tmp[1]
203 try:
204 with open(tmpfname, 'wb') as fp:
205 fp.write(test_data)
206 event = events.FinishReportingEvent('test_event_name',
207 'test event description',
208 post_files=[tmpfname],
209 level='INFO')
210 webhook_handler = handlers.WebHookHandler('127.0.0.1:8000',
211 level='INFO')
212 webhook_handler.publish_event(event)
213 webhook_handler.oauth_helper.geturl.assert_called_with(
214 url='127.0.0.1:8000', data=event.as_dict(),
215 headers=webhook_handler.headers, retries=None)
216 finally:
217 os.remove(tmpfname)

Subscribers

People subscribed via source and target branches