Skip to content
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

Allow to implement caching in instrumentation #5

Merged
merged 1 commit into from Sep 15, 2012

Conversation

pranas
Copy link
Contributor

@pranas pranas commented Sep 10, 2012

I know this change breaks your API but it gives more power to instrument_request method. You can use it to implement caching for example.

Please consider this change.

@Sutto
Copy link
Owner

Sutto commented Sep 15, 2012

I'd honestly rather introduce a method to do this, versus overriding the instrumentation to do so. That way we can maintain backwards compatibility but more importantly we're not using the instrumentation hooks to implement something that isn't instrumentation-related.

That said, I do like the change since it does give a little bit more power in instrumentation (namely, instrumenting on the return data as well)

Sutto added a commit that referenced this pull request Sep 15, 2012
Allow to implement caching in instrumentation
@Sutto Sutto merged commit c54cd7f into Sutto:master Sep 15, 2012
@pranas
Copy link
Contributor Author

pranas commented Sep 15, 2012

I totally agree with you. Dedicated hook for caching purposes would be great.

I now think that this change is not that good after all. Besides that it breaks backwards compatibility, it lets people abuse instrument_request method.

What do you mean by "instrumenting on the return data"?

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

Successfully merging this pull request may close these issues.

None yet

2 participants