-
Notifications
You must be signed in to change notification settings - Fork 814
Add handler parameter to pushgateway functions to permit HTTP AUTH etc. #120
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
Conversation
Allow a custom handler to be provided, so that the caller can provide code which carried out basic auth, https client certificate validation or other arbitrary schemes and access methods such as using different types of proxy.
Provide pydoc for the new handler function to the various pushgateway functions, provide parameter descriptions in the documentatation for the push_to_gateway function to avoid excessive copy-and-paste.
Bah, trying to rush it at the end of the day. Will fix Sunday if I get a chance. Sorry for the noise, please ignore for now... |
raise IOError("error talking to pushgateway: {0} {1}".format( | resp.code, resp.msg)) | |
else: | ||
handler(url=url, method=lambda: method, timeout=timeout, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a lambda for the method?
will be carried out by a default handler. | ||
If not None, the argument must be a function which accepts | ||
the following arguments: | ||
url, method, timeout, headers, and content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be good to include an example one doing something simple like basic auth
raise IOError("error talking to pushgateway: {0} {1}".format( | ||
resp.code, resp.msg)) | ||
if handler is None: | ||
resp = build_opener(handler).open(request, timeout=timeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's changed HTTPHandler to handler, which will be None. Is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks off alright. Can you send a PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sent in PR #126.
Superseded by #126 |
First stab at a handler implementation. No tests yet, please let me know what you think. Documenting a BASIC AUTH handler would probably be a good idea as per #60