-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
RFC: Add support for "all except x" in JULIA_DEBUG #32432
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me.
Done!
I've updated the tests. I've also updated the examples in the original post. |
It doesn’t look like the buildbot failures are related to this PR. |
Done! |
This is a nice bit of functionality to have! @c42f, would you be willing to review? |
@StefanKarpinski sure. The functionality here is useful, but I still don't think we should have the As a possible way forward, how about we consider moving all |
As I see it the problem is that other than |
Apologies for being short / cryptic, I'm just busy. I do appreciate that people care for the usability and I know the current system is difficult to control as it comes "out of the box". However, lacking the time to think deeply about this I'd rather leave the innovation to packages. My specific concern with Instead I'd prefer to hoist all this logic into the loggers so that logging control is at least thread local and can't be modified by a callee. Specifically I think we should
|
Ugh, my apologies - I definitely didn't mean to close this. Nor was I finished replying... The above notes are reasonably representative of my thoughts though, if a bit rough around the edges. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On further consideration I think we should just merge this PR: it's undeniably useful functionality and doesn't change anything fundamental about the way that JULIA_DEBUG
is used. A further PR can always refactor a bit to improve the performance and preserve the use case which this serves.
One question: how committed are we to an environment variable being the official API here? I'm still hazy about how stable we require the stdlibs to be and I originally thought of JULIA_DEBUG
as simply a useful internal hack for debugging Base during early initialization...
Not at all. If there were a better API people we could encourage people to use it. |
Bump. Is there a consensus in favor of merging this PR? If so, do you think it can make it into Julia 1.4? |
Out of curiosity, what is the use case for this? It seems this is basically saying, "everyone, feel free to spew as much debug messages you want at me, except this guy, you should be quiet". When does that actually happen? |
Yeah good point. For me, the use case is that I'm debugging some application with a whole bunch of modules and dependencies, and I'm trying to nail down exactly where things go wrong. There are so many Sometimes, though, there are one or two modules that (1) I know are not the source of the bug, and (2) spew a lot of debug messages. In those cases, it's helpful for me to disable debug messages from those modules only, which makes it easier for me to read the logs. |
Despite my comment above, I'm still struggling to decide what to do with this as it is. On the one hand it's useful, but I'd feel a lot more comfortable merging it if all the environment variable logic was moved into a helper function and called from Do you have time to invest on following this up with a PR which does that? The key is that users should be able to opt out of the entire code path which looks at |
Honestly, I don't think that I'll have time to work on that PR, at least not in the near future :( Regarding this PR, how about we close it for now, and revisit it in the future? |
Makes sense to me. That aligns pretty well with the original goal of letting you enable them easily based on lots of different criteria. |
Ok, well I guess we've got forward progress here (good!) but somewhat at the expense of communication / consensus (not so good :-/ ) @vtjnash I do feel like the conversation was truncated here and it would have been nice if you'd added a comment along the lines of "I'm going to merge this because I think forward progress in this case is better than worrying about the details". At least, I assume that's the judgement call you're making here, and I think it's fair enough. I need to do a design iteration on log record dispatch anyway, so perhaps this will spur it along 😬 |
If I want to see debugging messages for the module
Foo
, I can set theJULIA_DEBUG
environment variable toloading
.If I want to see debugging messages from all modules, I can set the
JULIA_DEBUG
environment variable toall
.However, if I want to see all debugging messages EXCEPT those from the
Foo
module, currently there is no way of doing this in Julia.This pull request adds the ability to turn on debugging messages from all modules except a specified list. The modules to exclude are prefixed with
!
when listed inJULIA_DEBUG
. For example,ENV["JULIA_DEBUG"] = "all,!Foo,!Baz"
will turn on debugging messages from all modules exceptFoo
andBaz
.The existing behavior of
JULIA_DEBUG
is preserved.Example usage
Summary
JULIA_DEBUG
Foo
Bar
Baz
Foo
Foo,Baz
all
all,!Foo
!Foo
!Foo,!Baz
all,!all
all,!all,!Foo
all,!all,!Foo,!Baz
!all,!Foo
!all,!Foo,!Baz
all,!all,!Foo,Bar,!Baz
!all,!Foo,Bar,!Baz
Code
If
!all
is specified, it "cancels out"all
.If neither
all
nor!all
are specified, then!x
impliesall
.If
!all
is specified, then!x
does not implyall
.