Add handler parameter to pushgateway functions to permit HTTP AUTH etc. (refs issue #60) #126

rossigee · 2017-01-15T07:02:23Z

Takes the branch by @tim-seoss (PR #120), fixes a minor regression, moves the default handler code out to be more easily re-used/adapted, and adds a basic unit test for a custom handler.

A simple handler function for adding Basic Auth might then look like this...

#!/usr/bin/python

import os, base64

from prometheus_client.handlers.base import handler as default_handler

def handler(url, method, timeout, headers, data):
    '''Handler that implements HTTP Basic Auth using environment variables 'PUSHGW_USERNAME' and 'PUSHGW_PASSWORD'.'''
    username = os.environ['PUSHGW_USERNAME']
    password = os.environ['PUSHGW_PASSWORD']
    auth_value = "{0}:{1}".format(username, password)
    auth_header = "Basic {0}".format(base64.b64encode(bytes(auth_value)))
    headers.append(['Authorization', auth_header])
    default_handler(url, method, timeout, headers, data)

@brian-brazil - your thoughts?

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.

… handler (refs #120)

rossigee · 2017-01-15T09:46:33Z

I found I needed to pass additional arguments to the handler, such as the username/password, so I have updated the patch to include a 'handler_args' parameter, and included a simple 'basic_auth' handler too.

Usage then becomes something like this...

from prometheus_client.handlers.basic_auth import handler as http_basic_auth_handler
auth_args = None
if username is not None:
    auth_args = {
        'username': username,
        'password': password
    }

push_to_gateway(url, job=job, registry=registry, handler=http_basic_auth_handler, handler_args=self.auth_args)

brian-brazil · 2017-01-16T17:32:37Z

Can you rebase this?

I'd have presumed that any args are embedded in the handler, so there's no need to pass args beyond that.

rossigee · 2017-01-17T06:15:53Z

Rebased.

Re: 'args embedded in the handler' - I'm not sure what you mean or how to do that. The only way other way I could think to minimise the footprint was to change it from a function to an object, but that didn't feel right for some reason, so I used the 'kwargs' approach I've seen used elsewhere. My python isn't that sharp though, so I'm open to better suggestions.

Anyway, have now tested this branch more widely with several clients and they are now pushing data happily to push gateway services that are using https+basicauth, so I can at least report it works for me.

Also, I've just seen the py26 travis build failure (URLError), but I can't seem to figure out wtf would cause that, other than perhaps a python26/urllib2 bug. I can't see how it would be related to this patch, unless I'm missing something?

brian-brazil · 2017-01-17T08:30:00Z

That's still not rebased.

Re: 'args embedded in the handler' - I'm not sure what you mean or how to do that.

Use a closure. So instead of:

def func(a): return a
func(x)

do

def func(): return x
func()

This works with classes too.

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.

… handler (refs #120)

rossigee · 2017-01-25T05:01:56Z

@brian-brazil - ok, i think i've got it - how does it look now?

brian-brazil · 2017-01-25T09:25:07Z

README.md

+def my_auth_handler(url, method, timeout, headers, data):
+    username = os.environ['PUSHGW_USERNAME']
+    password = os.environ['PUSHGW_PASSWORD']
+    return basic_auth_handler(url, method, timeout, headers, data, username, password)


I'd hardcode the user/pw here to keep it simpler.

If you use an env example, some users will be confused and think it only works if it comes from the env.

Example updated.

brian-brazil · 2017-01-25T09:26:58Z

prometheus_client/exposition.py

-        raise IOError("error talking to pushgateway: {0} {1}".format(
-            resp.code, resp.msg))
+    headers=[('Content-Type', CONTENT_TYPE_LATEST)]
+    if handler is None:


The handler isn't mutable, so you can put it as a default in the def line.

Not sure I can see how that would work, as the function is called with a handler of None by other functions in exposition.py. Do you mean make 'default_handler' the default instead of None for all of them?

Do you mean make 'default_handler' the default instead of None for all of them?

Yes

brian-brazil · 2017-01-25T09:27:46Z

prometheus_client/exposition.py


 from . import core
+
+from .handlers.base import handler as default_handler


I'd put these in this file to keep things all in one place.

Not sure I understand. Do you mean something like...

from .handlers.base import default_handler

and in base.py...

default_handler = handler

or something else???

I mean put all of this in exposition.py

brian-brazil · 2017-01-25T09:28:27Z

prometheus_client/handlers/basic_auth.py

+
+def handler(url, method, timeout, headers, data, username = None, password = None):
+    def handle():
+        '''Handler that implements HTTP Basic Auth by setting auth headers if


The first sentence in a docstring should be on one line.

brian-brazil · 2017-01-25T17:11:00Z

prometheus_client/exposition.py



-def push_to_gateway(gateway, job, registry, grouping_key=None, timeout=None):
+def default_handler(url, method, timeout, headers, data):


Can you add a docstring to indicate this is for use with the pgw functions?

brian-brazil · 2017-01-25T17:11:34Z

prometheus_client/handlers/basic_auth.py

+
+from prometheus_client.exposition import default_handler
+
+def handler(url, method, timeout, headers, data, username = None, password = None):


This should go in exposition.py too.

No spaces around the = here

brian-brazil · 2017-01-26T10:50:39Z

The unittests aren't passing on all version of Python, could you fix that up and we can get this submitted?

…in python3).

…ing issue.

This reverts commit 62e8012.

rossigee · 2017-01-29T06:41:42Z

I'm not having much luck fixing the python2.6 test. I have no py26 environment locally to reproduce it with, and from what I can determine from the error/stacktrace, it's more of an issue with python2.6's urllib2 implementation on the server than it is a regression caused by this patch, unless you can see something that I'm missing.

brian-brazil · 2017-01-30T14:20:46Z

Doing some testing, it seems to be the lack of a http:// on the url that's causing the issue.

rossigee · 2017-01-31T02:22:31Z

That's the same conclusion I came to, hence the last patch and revert. I've just tried another workaround, which seems to have cleared it now. Anyway, I couldn't see anything in the patch that would affect that behaviour, so I'm guessing something changed in the py26 test environment since the test was passing. I'd be interested to see if the master branch (unpatched) still passes that test.

brian-brazil · 2017-01-31T11:29:24Z

Thanks!

tim-seoss and others added 5 commits November 23, 2016 18:12

Just check for a url scheme to allow users to provide a handler.

6e1e16a

Document new handler parameter to pushgateway function

7b0677c

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.

Merge branch 'master' of https://github.com/prometheus/client_python

0be1ec5

Separate default handler code for re-use and add test case for custom…

c37a15c

… handler (refs #120)

rossigee mentioned this pull request Jan 15, 2017

Add handler parameter to pushgateway functions to permit HTTP AUTH etc. #120

Closed

rossigee added 3 commits January 15, 2017 15:26

Fix relative import error.

5914671

Add 'handlers' package to distribution.

0c1220a

Add 'handler_args' to pass extra informatoin to handler.

d8a1694

rossigee added 2 commits January 17, 2017 12:09

Merge branch 'master' of https://github.com/prometheus/client_python

a67bf46

Remove unnecessary imports.

51b1ab0

tim-seoss and others added 11 commits January 25, 2017 10:35

Just check for a url scheme to allow users to provide a handler.

e3071c4

Document new handler parameter to pushgateway function

568f6ca

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.

Separate default handler code for re-use and add test case for custom…

814e24d

… handler (refs #120)

Fix relative import error.

9139e82

Add 'handlers' package to distribution.

ea1e3e7

Add 'handler_args' to pass extra informatoin to handler.

0ac7442

Remove unnecessary imports.

d3f5a58

Merge branch 'master' of https://github.com/rossigee/client_python

8adfe8a

Use closure to pass args to handlers (refs #60).

44df258

Add missing import to auth example.

46a9e92

brian-brazil reviewed Jan 25, 2017

View reviewed changes

rossigee added 2 commits January 25, 2017 16:52

Simplify example by hardcoding username/password.

eb9500b

Fix for docstring comment.

7f863dd

brian-brazil mentioned this pull request Jan 25, 2017

Add HTTPRedirectHandler for pushing to gateway #131

Closed

Move base handler back into exposition.

0bebb0a

brian-brazil reviewed Jan 25, 2017

View reviewed changes

Move basic auth handler into exposition. Update docstring.

7e45925

rossigee added 6 commits January 29, 2017 12:56

Fix for 'TypeError: string argument without an encoding' (regressing …

daa40fd

…in python3).

Fix for 'TypeError: string argument without an encoding' (regressing …

1ba7c2e

…in python3).

Fix for 'TypeError: string argument without an encoding' (regressing …

8f7511a

…in python3).

Resort to determining python version to work around base64 byte encod…

444a748

…ing issue.

Potential workaround for py26 test failures.

62e8012

Revert "Potential workaround for py26 test failures."

b084d64

This reverts commit 62e8012.

Another potential workaround for py26 test failures.

a7afc96

brian-brazil closed this Jan 31, 2017

brian-brazil reopened this Jan 31, 2017

brian-brazil merged commit aa4b4fb into prometheus:master Jan 31, 2017


		from . import core

		from .handlers.base import handler as default_handler



		def push_to_gateway(gateway, job, registry, grouping_key=None, timeout=None):
		def default_handler(url, method, timeout, headers, data):


		from prometheus_client.exposition import default_handler

		def handler(url, method, timeout, headers, data, username = None, password = None):

Add handler parameter to pushgateway functions to permit HTTP AUTH etc. (refs issue #60) #126

Add handler parameter to pushgateway functions to permit HTTP AUTH etc. (refs issue #60) #126

Uh oh!

Conversation

rossigee commented Jan 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rossigee commented Jan 15, 2017

Uh oh!

brian-brazil commented Jan 16, 2017

Uh oh!

rossigee commented Jan 17, 2017

Uh oh!

brian-brazil commented Jan 17, 2017

Uh oh!

rossigee commented Jan 25, 2017

Uh oh!

brian-brazil Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brian-brazil commented Jan 26, 2017

Uh oh!

rossigee commented Jan 29, 2017

Uh oh!

brian-brazil commented Jan 30, 2017

Uh oh!

rossigee commented Jan 31, 2017

Uh oh!

brian-brazil commented Jan 31, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rossigee commented Jan 15, 2017 •

edited

Loading

brian-brazil Jan 25, 2017 •

edited

Loading