Issue 4490: Added thread name to IO.debug #4525
Conversation
|
Thanks for the PR. I'm wondering, what is the intention here? To print which thread exactly? |
|
@durban Not sure I understand your question, |
|
Sure, but which thread, executing what code, when? The thread writing to stdout? The thread executing |
|
The thread in |
|
The intent is to add thread name to |
djspiewak
left a comment
djspiewak
left a comment
There was a problem hiding this comment.
So asking @durban's question a slightly different way, it's not clear why you need the name of the carrier thread. Can you explain the use-case a bit more here? What are you trying to determine by printing this out? I genuinely have no objections to it (once the implementation is fixed), but I don't really get what you're trying to accomplish.
Cats Effect moves fibers freely between carrier threads depending on a whole host of very complex factors. In non-degenerate cases, it shouldn't matter what the underlying thread is. I definitely get that, particularly with custom thread pools, you may have a scenario where this is required, but I'd like to understand that better.
| case Outcome.Succeeded(ioa) => | ||
| ioa.flatMap(a => IO.println(s"${prefix}: Succeeded: ${S.show(a)}")) | ||
| ioa.flatMap(a => | ||
| IO.println(s"[${Thread.currentThread().getName}] ${prefix}: Succeeded: ${S.show(a)}")) |
There was a problem hiding this comment.
Thread.currentThread() is a side effect and should be wrapped in IO. The reason for this becomes much more apparent when you look at the output for the Canceled case in particular, which will frequently be inconsistent with the thread on which the IO is actually executing!
There was a problem hiding this comment.
It’d be helpful for the same reason prefix exists, debugging.
4 participants
An implementation for #4409