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

Bundle decorator.py and fix decorators #91

Merged
merged 1 commit into from Jul 20, 2016

Conversation

Projects
None yet
3 participants
@hynek
Contributor

hynek commented Jul 20, 2016

As discussed in #77, this PR bundles (to avoid dependencies) the decorator.py module and uses it to fix the behavior of prometheus_client's decorators.

The efficacy can be verified by using example view from the README and then doing a:

>>> from inspect import getargspec
>>> getargspec(process_request)
ArgSpec(args=['t'], varargs=None, keywords=None, defaults=None)

Without this patch, you get:

ArgSpec(args=[], varargs='args', keywords='kwargs', defaults=None)

instead.

Let me now what you think.

Also pinging @micheles out of courtesy and to ensure he doesn’t feel we violate his copyright.

@brian-brazil

This comment has been minimized.

Show comment
Hide comment
@brian-brazil

brian-brazil Jul 20, 2016

Member

That looks good, could you add a quick unittest so we don't accidentally regress?

Member

brian-brazil commented Jul 20, 2016

That looks good, could you add a quick unittest so we don't accidentally regress?

@micheles

This comment has been minimized.

Show comment
Hide comment
@micheles

micheles Jul 20, 2016

Yes, no problems on my part, the decorator module is a single file just to
help people that wants to include it and have less dependencies.

On Wed, Jul 20, 2016 at 3:17 PM, Hynek Schlawack notifications@github.com
wrote:

As discussed in #77
#77, this PR bundles
(to avoid dependencies) the decorator.py module and uses it to fix the
behavior of prometheus_client's decorators.

The efficacy can be verified by using example view from the README and
then doing a:

from inspect import getargspec
getargspec(process_request)
ArgSpec(args=['t'], varargs=None, keywords=None, defaults=None)

Without this patch, you get:

ArgSpec(args=[], varargs='args', keywords='kwargs', defaults=None)

instead.

Let me now what you think.

Also pinging @micheles https://github.com/micheles out of courtesy and

to ensure he doesn’t feel we violate his copyright.

You can view, comment on, or merge this pull request online at:

#91
Commit Summary

  • Bundle decorator.py and fix decorators

File Changes

Patch Links:


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#91, or mute the thread
https://github.com/notifications/unsubscribe-auth/ACWYcHT25T7oA9sc3uu03o9FP98mtKYiks5qXh_rgaJpZM4JQw7E
.

micheles commented Jul 20, 2016

Yes, no problems on my part, the decorator module is a single file just to
help people that wants to include it and have less dependencies.

On Wed, Jul 20, 2016 at 3:17 PM, Hynek Schlawack notifications@github.com
wrote:

As discussed in #77
#77, this PR bundles
(to avoid dependencies) the decorator.py module and uses it to fix the
behavior of prometheus_client's decorators.

The efficacy can be verified by using example view from the README and
then doing a:

from inspect import getargspec
getargspec(process_request)
ArgSpec(args=['t'], varargs=None, keywords=None, defaults=None)

Without this patch, you get:

ArgSpec(args=[], varargs='args', keywords='kwargs', defaults=None)

instead.

Let me now what you think.

Also pinging @micheles https://github.com/micheles out of courtesy and

to ensure he doesn’t feel we violate his copyright.

You can view, comment on, or merge this pull request online at:

#91
Commit Summary

  • Bundle decorator.py and fix decorators

File Changes

Patch Links:


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#91, or mute the thread
https://github.com/notifications/unsubscribe-auth/ACWYcHT25T7oA9sc3uu03o9FP98mtKYiks5qXh_rgaJpZM4JQw7E
.

@hynek

This comment has been minimized.

Show comment
Hide comment
@hynek

hynek Jul 20, 2016

Contributor

I’ve added unit tests for hopefully all decorators and the tests still pass.

Contributor

hynek commented Jul 20, 2016

I’ve added unit tests for hopefully all decorators and the tests still pass.

@brian-brazil brian-brazil merged commit cba7100 into prometheus:master Jul 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brian-brazil

This comment has been minimized.

Show comment
Hide comment
@brian-brazil

brian-brazil Jul 20, 2016

Member

Thanks!

Member

brian-brazil commented Jul 20, 2016

Thanks!

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