hipchat support #324
hipchat support #324
Conversation
src/iris/vendors/iris_hipchat.py
Outdated
| self.notification_url = '{0}/v2/room/{1}/notification'.format(self.endpoint_url, self.room_id) | ||
| self.headers = { | ||
| 'Content-type': 'application/json', | ||
| } |
There was a problem hiding this comment.
slight nitpick: can you put the closing brace on the same level of indention as self.headers?
src/iris/vendors/iris_hipchat.py
Outdated
| def send_message(self, message): | ||
| start = time.time() | ||
| payload = ujson.dumps(self.get_message_payload(message)) | ||
| logger.info('tmpdebug msg: %s', ujson.dumps(message)) |
There was a problem hiding this comment.
should remove or move this logger.info to the if self.debug section, or take it out of the PR and just leave it uncommitted for local testing (what we do in this situation)
src/iris/vendors/iris_hipchat.py
Outdated
| 'notify': 'true', | ||
| 'message_format': "text", | ||
| } | ||
| return self.message_dict |
There was a problem hiding this comment.
probably should avoid setting message_dict as a property of the class, and just return it.
that way the function becomes pure and doesn't modify external state (makes testing cleaner and more reproducible) and won't ever cause problems if we decide to call the same vendor object more than once concurrently
src/iris/vendors/iris_hipchat.py
Outdated
| response = requests.post(self.notification_url, | ||
| headers=self.headers, | ||
| params=self.params, | ||
| data=payload, |
There was a problem hiding this comment.
you can avoid manually making the json string and passing it to data= and instead just do json=payload and it will encode in json for you
test/test_iris_vendor_hipchat.py
Outdated
| # -*- coding:utf-8 -*- | ||
|
|
||
| from iris.vendors.iris_hipchat import iris_hipchat | ||
| import ujson as json |
There was a problem hiding this comment.
this json import probably can be removed as it isn't used anywhere in this file
- do json handling in request call - make message_dict local - cleanups
src/iris/vendors/iris_hipchat.py
Outdated
| start = time.time() | ||
| payload = self.get_message_payload(message) | ||
| if self.debug: | ||
| logger.info('debug: %s', self.message_dict) |
There was a problem hiding this comment.
I think self.message_dict here should be payload?
src/iris/vendors/iris_hipchat.py
Outdated
|
|
||
|
|
||
| class iris_hipchat(object): | ||
| supports = frozenset(['hipchat']) |
There was a problem hiding this comment.
Can we add a HIPCHAT_SUPPORT = 'hipchat' to constants.py and use that here and in the iris.constants import above?
fix flake8 tests Change-Id: Iee64ccf07aee39a7b0aa72d5e6e89e1ae643eb1b
|
All comments addressed and tests fixed. |
|
Thanks! |
|
Whoops meant to squash rather than rebase |
Messages will be posted to the provided room id.
Users will be 'mentioned', enabling them to receive their alerts.
#321
Known issue: currently the iris username is used as the hipchat username when sending alerts. I'll make this configurable once I understand how the hipchat username field can be stored in Oncall.