Add ability to customize worker prefix#157
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #157 +/- ##
==========================================
- Coverage 79.47% 79.19% -0.29%
==========================================
Files 10 10
Lines 1959 1966 +7
==========================================
Hits 1557 1557
- Misses 402 409 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The worker_output_hook API documents that ident is passed as AbstractString, but the connect function in managers.jl was passing the pid as an Int. This caused hooks that expected strings (e.g., using tryparse(Int, ident)) to fail silently. Match the behavior in cluster.jl which already passes "$pid".
IanButterworth
changed the title
JamesWrigley
left a comment
JamesWrigley
left a comment
There was a problem hiding this comment.
Related PR: JuliaParallel/DistributedNext.jl#17
| end | ||
| ``` | ||
| """ | ||
| const worker_output_hook = Ref{Union{Nothing, Function}}(nothing) |
There was a problem hiding this comment.
I don't think I like that we expect the handler to fully take over the role of printing stuff as well. We may very well change it in the future to better support logging or something, so I would prefer that the callback instead be a transform that takes in the string and returns a (possibly different) string for Distributed to handle in its own way.
In which case we could have:
| const worker_output_hook = Ref{Union{Nothing, Function}}(nothing) | |
| worker_output_hook::Function = identity |
|
Regarding the design in JuliaParallel/DistributedNext.jl#17, I'm not sure if we should support per-module callbacks. I could maybe see it being useful if a higher-level tool like Dagger wanted to customize the output a bit more (CC @jpsamaroo). |
|
I would also be ok with keeping this a strictly internal feature for the Julia tests 🤷 |
|
If we keep it internal and non-public, then we have the added benefit of being able to make breaking changes to it (or even remove it entirely) in the future. |
3 participants
For JuliaLang/julia#60372