By default, propagate Base.active_project(), Base.LOAD_PATH, and Base.DEPOT_PATH to the workers (but allow the user to override that behavior by specifying JULIA_{PROJECT,LOAD_PATH,DEPOT_PATH} in params[:env])
#30
Conversation
DilumAluthge
changed the title
Base.active_project(), Base.LOAD_PATH, and Base.DEPOT_PATH to the workers (but allow the user to override that behavior by specifying JULIA_{PROJECT,LOAD_PATH,DEPOT_PATH} in params[:env]Base.active_project(), Base.LOAD_PATH, and Base.DEPOT_PATH to the workers (but allow the user to override that behavior by specifying JULIA_{PROJECT,LOAD_PATH,DEPOT_PATH} in params[:env])
|
Ping on this as well. @kleinhenz could this be merged when you have a chance? I'm really eager to try this out for PySR. |
|
I think I'll need to rebase this, but after that I think this should be good to go. I'll try to get to this later this week. |
|
Alright, I've rebased this one. @kleinhenz This one should be ready for review now. |
src/slurmmanager.jl
Outdated
| user_did_specify_JULIA_PROJECT = false | ||
| user_did_specify_JULIA_LOAD_PATH = false | ||
| user_did_specify_JULIA_DEPOT_PATH = false | ||
|
|
||
| for (name, value) in pairs(params_env) | ||
| # For each key-value mapping in `params[:env]`, we respect that mapping and we pass it | ||
| # to the workers. | ||
| env2[name] = value |
There was a problem hiding this comment.
nit: I think this would be slightly clearer if we refactored to use merge and haskey instead of explicitly looping over keys.
There was a problem hiding this comment.
I've done some refactoring (0bde517). I got rid of the loop over key-value pairs. Instead of using merge() with an empty Dict, I just did a copy(). However, if desired, I'm happy to instead construct an empty Dict and then do a merge().
And I added haskey()s for the places where we check if the keys exist.
|
@kleinhenz Would you mind making a new release of SlurmClusterManager.jl before we merge this PR? That way, if this PR breaks anyone's existing workflows, they can just pin to the previous release. |
|
I'll convert to draft just so that we don't accidentally merge this until we make a release first. |
|
Just made a new release. |
…`Base.DEPOT_PATH` to the workers (but allow the user to override that behavior by specifying `JULIA_{PROJECT,LOAD_PATH,DEPOT_PATH}` in `params[:env]`)
|
Alright, @kleinhenz this is ready for another round of review. |
|
Bump @kleinhenz |
|
looks good to me, thanks! |
|
Thanks so much @kleinhenz! Can you register a new release? |
Fixes #16.
Alternative to #17 (closes #17).
Alternative to #19 (closes #19).
Depends on:
params[:env](that is, theenvkwarg to theaddprocs()function) #20