-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[autoscaler] Submit command #3312
Conversation
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
python/ray/autoscaler/commands.py
Outdated
@@ -236,6 +236,55 @@ def attach_cluster(config_file, start, use_tmux, override_cluster_name, new): | |||
override_cluster_name, None) | |||
|
|||
|
|||
def submit_cluster(config_file, screen, tmux, stop, start, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is just sugar for rsync_up followed by an exec, why not just call those two functions instead of making this new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, rsync_up
doesn't have start
cluster functionality but exec
has start
cluster functionality.
Ideally, you'd start
, rsync
, and exec
.
One alternative is to do create_or_update_cluster
, rsync
, and then exec
but it wasn't clear to me that was the cleanest way of doing things (it would create 3 different NodeUpdater
s)
I think doing that is fine, even if it's a bit slower. The code will be
hard to maintain otherwise.
…On Tue, Nov 13, 2018, 6:11 PM Richard Liaw ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In python/ray/autoscaler/commands.py
<#3312 (comment)>:
> @@ -236,6 +236,55 @@ def attach_cluster(config_file, start, use_tmux, override_cluster_name, new):
override_cluster_name, None)
+def submit_cluster(config_file, screen, tmux, stop, start,
Hm, rsync_up doesn't have start cluster functionality but exec has start
cluster functionality.
Ideally, you'd start, rsync, and exec.
One alternative is to do create_or_update_cluster, rsync, and then exec
but it wasn't clear to me that was the cleanest way of doing things (it
would create 3 different NodeUpdaters)
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#3312 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAA6Soo1MLIFiCv-jZRhih5oJmBSA3snks5uu3vQgaJpZM4Ya2JN>
.
|
Test PASSed. |
As requested from users.
TODO:
Changes
This allows users to submit scripts and args to a cluster (that might not even exist yet). Closes #2970.