Skip to content

Add https support for monit collector. #543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add https support for monit collector. #543

wants to merge 3 commits into from

Conversation

nesoneg
Copy link

@nesoneg nesoneg commented Oct 5, 2016

Use private methods, but worked.

@shortdudey123
Copy link
Member

Can you fix the pep8 errors? also update the doc and testing?

int(self.config['port']))

if self.config['selfsigned'] and sys.hexversion >= 0x020709f0 and hasattr(ssl, '_create_unverified_context'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment as to what version 0x020709f0 correlates to? or use sys.version_info instead?

More compliance PEP8 format source code.
Domumentation update.
@nesoneg
Copy link
Author

nesoneg commented Oct 6, 2016

I update doc and bit more accurate PEP8.
Tests i have not mastered…

@@ -21,6 +21,8 @@ measure_collector_time | False | Collect the collector run time in ms | bool
metrics_blacklist | None | Regex to match metrics to block. Mutually exclusive with metrics_whitelist | NoneType
metrics_whitelist | None | Regex to match metrics to transmit. Mutually exclusive with metrics_blacklist | NoneType
send_totals | False | Send cpu and memory totals | bool
scheme | http | Select scheme http or https | str
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetize

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The descriptions need to be added to the get_default_config_help method in the collector file since this doc is actually generated from that.

@shortdudey123
Copy link
Member

1 pep8 error left
src/collectors/monit/monit.py:56:81: E501 line too long (119 > 80 characters)1

For testing, PR540 would be a good reference for the scheme checking

@coveralls
Copy link

coveralls commented Oct 14, 2016

Coverage Status

Coverage remained the same at 59.386% when pulling 19aa876 on nesoneg:master into 175a4db on python-diamond:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants