[kubectl-plugin] Add rayjob yaml generation to ray job submit command #2644
Conversation
| } else if !info.Mode().IsRegular() { | ||
| return fmt.Errorf("Filename given is not a regular file. Failed with: %w", err) | ||
| } | ||
| // Take care of case where no file name |
There was a problem hiding this comment.
Not sure if this comment is incorrect, as the next line checks that the filename is not empty.
There was a problem hiding this comment.
Oh yes, it should be take care of case where there is a filename
| @@ -71,6 +82,8 @@ var ( | |||
| Submit ray job to ray cluster as one would using ray CLI e.g. 'ray job submit ENTRYPOINT'. Command supports all options that 'ray job submit' supports, except '--address'. | |||
| If RayCluster is already setup, use 'kubectl ray session' instead. | |||
|
|
|||
| If no ray job file is specified, one will be generated. | |||
There was a problem hiding this comment.
We don't generate yaml file; we only apply a default RayCluster object for users, right?
There was a problem hiding this comment.
We will generate a basic/ rayjob apply config and apply that.
There was a problem hiding this comment.
Okay, I changed the wording. Please take another look.
There was a problem hiding this comment.
Oh, I see. Here, "one" refers to "RayJob", not "ray job file", right? Wouldn't it be better to replace this word with something more explicit? At first, I thought "one" referred to "ray job file".
There was a problem hiding this comment.
Yea, one is suppose to refer to RayJob, but you're right, having something explicit is definitely better in case of confusion.
There should be anymore confusion from the wording.
chiayi
force-pushed
the
kubectl-plugin
branch
from
af91523 to
147b306
Compare
chiayi
force-pushed
the
kubectl-plugin
branch
2 times, most recently
from
ee04aaa to
99a45e8
Compare
chiayi
force-pushed
the
kubectl-plugin
branch
3 times, most recently
from
fe7d2cd to
e99e7d7
Compare
| WorkerMemory: options.workerMemory, | ||
| WorkerReplicas: options.workerReplicas, | ||
| // This is here to match the existing rayjob sample. | ||
| WorkerLifecyclePrestopExecComand: []string{"/bin/sh", "-c", "ray stop"}, |
There was a problem hiding this comment.
I don't think we need this, or at least we've been removing them in sample YAML
There was a problem hiding this comment.
Confirmed that the sample rayjob still work after this is removed.
chiayi
force-pushed
the
kubectl-plugin
branch
from
e99e7d7 to
bcc1edc
Compare
chiayi
force-pushed
the
kubectl-plugin
branch
from
bcc1edc to
7a5474d
Compare
| log.Fatalf("Failed to mark flag as required %v", err) | ||
| } | ||
|
|
||
| cmd.Flags().StringVar(&options.rayjobName, "name", "", "Name of the ray job that will be generated") |
There was a problem hiding this comment.
Any way we can consolidate these flags into a single place with the flags also used in kubectl ray create cluster?
There was a problem hiding this comment.
I don't know if it's possible, I think we can try sharing the flags in some way. For example, setting the "parent" command to have these flags and this way all the "child" commands will as well.
Why are these changes needed?
Allows user to not need a pre-written rayjob yaml to submit a rayjob.
Related issue number
Part of #2608
Checks