feat: Bottle integration #300
Conversation
37424a8 to
ff17437
Compare
* inspired by flask and pyramid framework integrations * doesn't contain any login handling logic
ff17437 to
243d260
Compare
There was a problem hiding this comment.
This seems great! I left some comments though. If you want to start writing docs for this, these PRs should give you a clue: https://github.com/getsentry/sentry-docs/pulls?utf8=%E2%9C%93&q=is%3Apr+author%3Auntitaker+is%3Aclosed+python
sentry_sdk/integrations/bottle.py
Outdated
| # to monkey patch route.call | ||
| old_match = self.router.match | ||
|
|
||
| def patched_match(self, *args, **kwargs): |
There was a problem hiding this comment.
All this patching is occuring within sentry_patched_wsgi_app, but it should occur outside of it. There's no reason to patch on every request unless I am missing something.
Fixing this should make the use of types.MethodType unnecessary as well.
There was a problem hiding this comment.
I agree the patching done in this way is not really nice and a bit confusing. I tried to patch the other functions outside of sentry_patched_wsgi_app and I was more or less successful.
Just instead of Route.call (which is a cached_property) I managed to patch Router._make_callback. The tests on my machine seem to work.
sentry_sdk/integrations/bottle.py
Outdated
| # if the request is gone we are fine not logging the data from | ||
| # it. This might happen if the processor is pushed away to | ||
| # another thread. | ||
| if request is None: |
There was a problem hiding this comment.
This can never be None, it will always be that singleton instance of bottle.LocalRequest.
In Flask this exists because we make a weakref of the current request object, such that when a user copies the scope explicitly to a new thread or even into a job queue, the original request data is still there as long as the request is still alive (and not the current request from a thread that probably doesn't have request data). I am not sure if we can get a request object without threadlocals from a LocalRequest, but it doesn't seem so.
There was a problem hiding this comment.
You're right. Thanks for the explanation.
|
Thanks for the review. I'll try to process all the comments shortly. Also thanks for the documentation hint/link. |
b04be8d to
cae0a96
Compare
test only 0.x version (not every bugfix)
cae0a96 to
9297aac
Compare
using handy `transaction_from_function` function
useless `request is None` comparison removed
`handled` should be False
|
Almost there.
|
nested patching converted into flat patching
reformatted via formatting script
|
I just pushed two other fixups. The first updates the patching a bit and the second should reformat the source code using |
|
LGTM, however I can't release without docs. If you need assistance let me know |
|
Thanks. I'll look into it. |
|
I just managed to prepare a pull request for the docs. see getsentry/sentry-docs#867 |
|
@shenek I would plan to merge + release this as-is. This is really great work.
Any updates here? |
|
Thanks. I tried to hook some sample application against a production sentry instance and it seems that it captures events.
I plan to do some more test during next week. |
|
Fantastic, I'll release tomorrow
…On Fri, Mar 29, 2019 at 11:30 PM Stepan Henek ***@***.***> wrote:
Thanks.
I tried to hook some sample application against a production sentry
instance and it seems that it captures events.
[image: sentry-captured]
<https://user-images.githubusercontent.com/2659336/55265663-94a6f180-5279-11e9-913c-880719a36d53.png>
[image: sentry-bottle-detail]
<https://user-images.githubusercontent.com/2659336/55265801-244ca000-527a-11e9-9cc0-54547534c19f.png>
I plan to do some more test during next week.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#300 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAzHxaE1MGLm4zm8pJpIIs23n23FWJfWks5vbpPqgaJpZM4cB_25>
.
|
|
@shenek so this is out as 0.7.9: https://sentry.io/for/bottle/ |
|
Super. Thank you very much for your help. |
After some struggle I was finally able to prepare this MR, which provides some basic integration with Bottle web framework.
I was mostly inspired by Flask and Pyramid integrations source code.
The code itself doesn't handle any session/login logic and it contains a few bottle specific stuff.
In a couple of days I plan to do some real life testing and I tend to prepare some kind of a tutorial.
Right now I will be grateful for any feedback or advice.