Skip to content
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

Runtime environment can specify Julia executable and arguments #8

Merged
merged 7 commits into from
Sep 14, 2023

Conversation

omus
Copy link
Member

@omus omus commented Sep 13, 2023

Working on beacon-biosignals/Ray.jl#103.

Something tricky about getting this to work was that I found that my serialized_runtime_env was set properly here but the RPC reply later had filtered out my new parameters. I wasn't sure where this was being processed so I removed env_vars from the RuntimeEnvContext and got a stack trace which pointed me to the dashboard code. It turns out that you need to pass through any runtime environment information here or it will be expected to be processed by a plugin. If no plugin is associated with the key then the value is just set to the default in RuntimeEnvContext.

# this information would be passed in via the serialized runtime
# context.
executable = self.executable or "julia"
args = self.args or ["-e", "'using Ray; start_worker()'"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we ever NOT want to do this? would it make more sense to allow users to pass extra arguments that are added in addition to these defaults?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see, that's passthrough_args

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can definitely drop the fallback but we probably want to keep it at least temporarily as otherwise we'll break Ray.jl CI when this is merged and beacon-biosignals/Ray.jl#103 has not yet been merged

python/ray/_private/runtime_env/context.py Outdated Show resolved Hide resolved
python/ray/_private/runtime_env/context.py Outdated Show resolved Hide resolved
dashboard/modules/runtime_env/runtime_env_agent.py Outdated Show resolved Hide resolved
@omus omus requested a review from kleinschmidt September 14, 2023 18:05
Copy link
Member

@kleinschmidt kleinschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy with these changes now. I think it's worth filing an issue to follow up on how the python executable is set (via a env var or something?) and possibly changing what we're doing here, but this unblocks us for now.

Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
@omus
Copy link
Member Author

omus commented Sep 14, 2023

happy with these changes now. I think it's worth filing an issue to follow up on how the python executable is set (via a env var or something?) and possibly changing what we're doing here, but this unblocks us for now.

beacon-biosignals/Ray.jl#121

@omus omus merged commit 0155184 into beacon-main Sep 14, 2023
@omus omus deleted the cv/driver-controlled-julia-cmd branch September 14, 2023 20:23
glennmoy pushed a commit that referenced this pull request Sep 27, 2023
* Support specifying runtime env executable

* Support specifying runtime env args

* Support specifying executable/args via runtime env

* Avoid quoting default

* Switch to using command in RuntimeEnvContext

* Use separate command for Julia

* Add TODO about switching to plugin

Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>

---------

Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
glennmoy pushed a commit that referenced this pull request Oct 27, 2023
* Support specifying runtime env executable

* Support specifying runtime env args

* Support specifying executable/args via runtime env

* Avoid quoting default

* Switch to using command in RuntimeEnvContext

* Use separate command for Julia

* Add TODO about switching to plugin

Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>

---------

Signed-off-by: Curtis Vogt <curtis.vogt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants