feat: Add debug log support in client init #115
Conversation
Codecov Report
@@ Coverage Diff @@
## master #115 +/- ##
=========================================
- Coverage 66.91% 66.7% -0.21%
=========================================
Files 25 25
Lines 2457 2526 +69
Branches 352 362 +10
=========================================
+ Hits 1644 1685 +41
- Misses 680 707 +27
- Partials 133 134 +1
Continue to review full report at Codecov.
|
|
What do you think about initializing integrations only when the client is bound to a hub or gets its first event? |
|
@untitaker initialize = call init_once? Probably not so much because that puts potential concurrency issues on the first request of a web app. |
|
Ok, then why not when the client is bound to a hub? Integrations are ineffective anyway if the client isn't bound to a hub. |
|
What's the benefit of doing that? I just worry this is going to make things worse for no obvious benefit. |
|
I'm just suggesting it as alternative to this one. I don't see drawbacks for either. |
|
So your proposal would be to remove the setup_integartions call in init and the context var, and have setup_integrations be called lazily on hub bind? |
|
Yes, but I don't know if it's a good idea. |
|
@mitsuhiko any opinion on my proposal? I don't really care anymore |
|
@untitaker i am considering it. i think debug logging is a bit stupid right now for more than one reason so it would be good to change it. eg: debug messages from the transport are suppressed. |
|
I don't think we can make a single API change that fixes both cases in case that's what you're aiming for. |
|
@untitaker no, but when making the logger explicit (eg: per hub) this PR becomes pointless. |
|
I'm not a big fan of this idea because it's incompatible with how we use
logging. dictConfig in sentry (where we explicitly install a sentry handler
on sentry_sdk.errors
…On Thu, Oct 11, 2018, 10:49 Armin Ronacher ***@***.***> wrote:
@untitaker <https://github.com/untitaker> no, but when making the logger
explicit (eg: per hub) this PR becomes pointless.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#115 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAzHxTTbbWakJI0CPGfd7IAKJNRMK9U3ks5ujwY1gaJpZM4XS-Za>
.
|
|
@mitsuhiko I'm actually fine with this as-is. Do you have any concerns? |
|
WFM. We can still clean it up later. It's just that it does not solve the issue that transports don't log. |
ebde09b to
73bd11f
Compare
No description provided.