Add client cert support and new action #39
Conversation
Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
Add cert config
Signed-off-by: Vivek Singh <[email protected]>
Add add watcher to issue command
Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
Replace double quotes with single quotes
added update_reporter action
There was a problem hiding this comment.
General comments:
- Thank you very much for this PR, overall looking good
- Please remove the "cosmetic" changes, it artificially inflates the size of the PR
- Please unit tests for the new functionality
- Please add a
CHANGELOG.mdentry with the updated changes - Please add these new actions and functionality (client certs) to the
README - Please bump the
versioninpack.yamlby0.1.0since new features were added.
Thanks again!
sensors/jira_sensor.py
Outdated
| super(JIRASensor, self).__init__(sensor_service=sensor_service, | ||
| config=config, | ||
| poll_interval=poll_interval) | ||
| super(JIRASensor, self).__init__( |
There was a problem hiding this comment.
Would prefer to keep cosmetic changes like this to a minimum, makes the PR artificially large. I notice there are several more as well.
| self._rsa_key = self._read_cert(rsa_cert_file) | ||
| self._poll_interval = self._config.get('poll_interval', self._poll_interval) | ||
| self._poll_interval = self._config.get( |
sensors/jira_sensor.yaml
Outdated
| type: "string" | ||
| issue_type: | ||
| type: "string" | ||
| class_name: "JIRASensor" |
There was a problem hiding this comment.
Looks like all cosmetic changes
| 'UpdateReporterValue' | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
Would like to see unit tests for these new actions.
There was a problem hiding this comment.
I don't see unit tests for individual action that are already existing. could you please help me understanding, what are you expecting from those unit tests, since most of these actions are connecting to JIRA API and returning result ?
| @@ -41,29 +42,39 @@ def setup(self): | |||
| self._jira_url = self._config['url'] | |||
| auth_method = self._config['auth_method'] | |||
|
|
|||
| options = {'server': self._config['url'], | |||
There was a problem hiding this comment.
Would like to see unit tests for these new options.
There was a problem hiding this comment.
I have already added these configurations in test fixtures which is being used my tests to run the test.
Signed-off-by: Vivek Singh <[email protected]>
Signed-off-by: Vivek Singh <[email protected]>
|
|
This PR adds support for client cert based authentication for the JIRA client.
It also adds tow new actions which are