JuliaLogging / LoggingExtras.jl

Composable Loggers for the Julia Logging StdLib
Other
142 stars 21 forks source link

`@debugv` fails on juliac branch #92

Open ericphanson opened 3 months ago

ericphanson commented 3 months ago
ERROR: LoadError: MethodError: Cannot `convert` an object of type LoggingExtras.Verbosity to an object of type Symbol
The function `convert` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  Symbol(::Any...)
   @ Base strings/basic.jl:229
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:126

Stacktrace:
  [1] macro expansion
    @ ./logging/logging.jl:427 [inlined]
  [2] iterate (repeats 2 times)
    @ ~/.julia/packages/Arrow/5pHqZ/src/table.jl:575 [inlined]
  [3] macro expansion
    @ ~/.julia/packages/Arrow/5pHqZ/src/table.jl:442 [inlined]
  [4] macro expansion
    @ ./task.jl:650 [inlined]
  [5] Arrow.Table(blobs::Vector{Arrow.ArrowBlob}; convert::Bool)
    @ Arrow ~/.julia/packages/Arrow/5pHqZ/src/table.jl:441
  [6] Table
    @ ~/.julia/packages/Arrow/5pHqZ/src/table.jl:415 [inlined]
  [7] Table (repeats 2 times)
    @ ~/.julia/packages/Arrow/5pHqZ/src/table.jl:407 [inlined]
  [8] read_arrow
    @ ~/.julia/packages/Legolas/7Ecrl/src/tables.jl:113 [inlined]
  [9] read(io_or_path::String; validate::Bool)
    @ Legolas ~/.julia/packages/Legolas/7Ecrl/src/tables.jl:160
 [10] read
    @ ~/.julia/packages/Legolas/7Ecrl/src/tables.jl:159 [inlined]

where the stacktrace points to https://github.com/apache/arrow-julia/blob/f1a91bfcbdca5532002d75c127db858d49133cec/src/table.jl#L446

on jb/gb/static-call-graph/f862b4f746. I can reproduce with just

julia> using LoggingExtras

julia> @debugv 1 "parsing schema message"
ERROR: MethodError: Cannot `convert` an object of type LoggingExtras.Verbosity to an object of type Symbol
The function `convert` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  Symbol(::Any...)
   @ Base strings/basic.jl:229
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:126

Stacktrace:
 [1] top-level scope
   @ REPL[4]:1
 [2] macro expansion
   @ logging/logging.jl:427 [inlined]

on that branch. Might be nothing to do here, since I think that branch has made some changes that might not be merged in exactly that form.

oxinabox commented 3 months ago

Oh yeah, I remember @topolarity on that branch restricted what a bunch of things could contain. In this case i guess group was restricted to Symbol which seems unfortunate, not just for this use case, but also for like i wanted to use a collection of tags as a group.

fredrikekre commented 3 months ago

See also https://github.com/JuliaLang/julia/pull/40820 for when I had code (in this repo?) that assumed Symbol

quinnj commented 3 days ago

Ah, interesting (sorry, missed this issue originally). My logging preferences have evolved somewhat recently where I'm actually likely to move a lot of my packages away from teh verbosity logging macros. I've become more interested in a core logging approach where each log is associated w/ a "subject", which I believe is what the group argument is for anyway. What I'd really like though, is a very easy/convenient way to not only globally set the log level, but also the subject (or subjects) I'm interested in.

For example, the verbosity logs don't really help when I turn on debug logging in HTTP/CSV, because I also get debug logs in the CodecZlib package, which I don't care about when I'm just trying to debug an HTTP/CSV issue. I'd rather be able to say that I'm specifically interested in a certain level + subject and only see logs tagged w/ that level/subject.

I think I owe an apology to @fredrikekre because I think he tried to dissuade me from the verbosity approach initially. Haha. Sorry it took me longer to come around and figure things out on my own.

quinnj commented 3 days ago

But with regards to the overall issue here, I guess we could generate special symbols that would be used here, like VERBOSITY_1, VERBOSITY_2, etc. and then have to check for those? It introduces a slight change for symbol collision (since anyone could potentially set the verbosity symbol themselves), but seems relatively safe.

quinnj commented 3 days ago

Alright, I think trying to support the verbosity macros is going to be too tedious, because we get into the tricky business of having to parse the VERBOSITY_N symbols to do the integer comparison on verbosity level. As I believe I'm the only user of these, I'm going to deprecate them.

In the same PR, I'm exporting withlevel (#81 ) and adding a convenience group keyword arg to withlevel that allows early filtering on the group arg of log messages. I think this will fit my ideal as mentioned above. I can go through my packages that use the verbosity logs and replace them with appropriate _group annotations which will pretty much get me all the way towards my ideal.

PR: https://github.com/JuliaLogging/LoggingExtras.jl/pull/94