8000 [Bug] Add default value for entrypoint flags in job_submit.go by 400Ping · Pull Request #3808 · ray-project/kuberay · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Bug] Add default value for entrypoint flags in job_submit.go #3808

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

400Ping
Copy link
Contributor
@400Ping 400Ping commented Jun 19, 2025 8000

Why are these changes needed?

Previously flags in ray job submit in those fields were not initialized to 0, nil, or "". This implicit approach can be confusing and risks future regressions if their initialization ever changes.
And I have create a test in pkg/cmd/job/job_submit_test.go to test my changes:

func TestRayJobSubmit_FlagsHaveDefaults(t *testing.T) {
	streams, _, _, _ := genericclioptions.NewTestIOStreams()
	factory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))
	opts := NewJobSubmitOptions(factory, streams)

	cmd := NewJobSubmitCommand(factory, streams)
	require.NoError(t, cmd.ParseFlags([]string{}))

	assert.InDelta(t, float32(0), opts.entryPointCPU, 1e-6, "default entrypoint-num-cpus should be 0")
	assert.InDelta(t, float32(0), opts.entryPointGPU, 1e-6, "default entrypoint-num-gpus should be 0")
	assert.Equal(t, 0, opts.entryPointMemory, "default entrypoint-memory should be 0")
	assert.False(t, opts.noWait, "default no-wait should be false")
}

Here are the results:

$ go test ./pkg/cmd/job -v
=== RUN   TestRayJobSubmitComplete
--- PASS: TestRayJobSubmitComplete (0.00s)
=== RUN   TestRayJobSubmitWithYamlValidate
=== RUN   TestRayJobSubmitWithYamlValidate/Successful_submit_job_validation_with_RayJob
=== RUN   TestRayJobSubmitWithYamlValidate/BackoffLimit_co-exist_with_InteractiveMode
=== RUN   TestRayJobSubmitWithYamlValidate/BackoffLimit_is_set_to_0_with_InteractiveMode
=== RUN   TestRayJobSubmitWithYamlValidate/shutdownAfterJobFinishes_is_false_and_ttlSecondsAfterFinished_is_not_zero
=== RUN   TestRayJobSubmitWithYamlValidate/shutdownAfterJobFinishes_is_true_and_ttlSecondsAfterFinished_is_not_zero
=== RUN   TestRayJobSubmitWithYamlValidate/ttlSecondsAfterFinished_is_less_than_zero
=== RUN   TestRayJobSubmitWithYamlValidate/ttlSecondsAfterFinished_is_less_than_zero#01
--- PASS: TestRayJobSubmitWithYamlValidate (0.01s)
    --- PASS: TestRayJobSubmitWithYamlValidate/Successful_submit_job_validation_with_RayJob (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/BackoffLimit_co-exist_with_InteractiveMode (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/BackoffLimit_is_set_to_0_with_InteractiveMode (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/shutdownAfterJobFinishes_is_false_and_ttlSecondsAfterFinished_is_not_zero (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/shutdownAfterJobFinishes_is_true_and_ttlSecondsAfterFinished_is_not_zero (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/ttlSecondsAfterFinished_is_less_than_zero (0.00s)
    --- PASS: TestRayJobSubmitWithYamlValidate/ttlSecondsAfterFinished_is_less_than_zero#01 (0.00s)
=== RUN   TestRayJobSubmitWithoutYamlValidate
=== RUN   TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_validate_value
=== RUN   TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_less_than_zero
=== RUN   TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_equal_to_zero
--- PASS: TestRayJobSubmitWithoutYamlValidate (0.00s)
    --- PASS: TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_validate_value (0.00s)
    --- PASS: TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_less_than_zero (0.00s)
    --- PASS: TestRayJobSubmitWithoutYamlValidate/ttlSecondsAfterFinished_is_equal_to_zero (0.00s)
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Both_shutdownAfterJobFinishes_and_ttlSecondsAfterFinished_are_not_set_in_yaml
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Both_shutdownAfterJobFinishes_and_ttlSecondsAfterFinished_are_set_in_yaml_with_wrong_values
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Only_shutdownAfterJobFinishes_is_set_in_yaml
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Only_ttlSecondsAfterFinished_is_set_in_yaml
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Override_only_ttlSecondsAfterFinished
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Set_ttl-seconds-after-finished_to_zero
=== RUN   TestRayJobSubmitCmdFlagsOverrideYaml/Override_only_ttlSecondsAfterFinished_to_negative_and_cause_error
--- PASS: TestRayJobSubmitCmdFlagsOverrideYaml (0.01s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Both_shutdownAfterJobFinishes_and_ttlSecondsAfterFinished_are_not_set_in_yaml (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Both_shutdownAfterJobFinishes_and_ttlSecondsAfterFinished_are_set_in_yaml_with_wrong_values (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Only_shutdownAfterJobFinishes_is_set_in_yaml (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Only_ttlSecondsAfterFinished_is_set_in_yaml (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Override_only_ttlSecondsAfterFinished (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Set_ttl-seconds-after-finished_to_zero (0.00s)
    --- PASS: TestRayJobSubmitCmdFlagsOverrideYaml/Override_only_ttlSecondsAfterFinished_to_negative_and_cause_error (0.00s)
=== RUN   TestDecodeRayJobYaml
--- PASS: TestDecodeRayJobYaml (0.00s)
=== RUN   TestRuntimeEnvHasWorkingDir
--- PASS: TestRuntimeEnvHasWorkingDir (0.00s)
=== RUN   TestRaySubmitCmd
--- PASS: TestRaySubmitCmd (0.00s)
=== RUN   TestRayJobSubmit_FlagsHaveDefaults
--- PASS: TestRayJobSubmit_FlagsHaveDefaults (0.00s)
PASS
ok      github.com/ray-project/kuberay/kubectl-plugin/pkg/cmd/job       1.205s

Related issue number

Closes #3807

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

400Ping added 2 commits June 19, 2025 23:14
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping 400Ping marked this pull request as draft June 19, 2025 15:51
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping 400Ping marked this pull request as ready for review June 19, 2025 16:09
@400Ping 400Ping changed the title [Bug] Add default-zero-value for entrypoint flags in job_submit.go [Bug] Add default-value for entrypoint flags in job_submit.go Jun 19, 2025
Signed-off-by: 400Ping <43886578+400Ping@users.noreply.github.com>
@400Ping 400Ping changed the title [Bug] Add default-value for entrypoint flags in job_submit.go [Bug] Add default value for entrypoint flags in job_submit.go Jun 19, 2025
@400Ping
Copy link
Contributor Author
400Ping commented Jun 19, 2025

@MortalHappiness PTAL

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.

[Bug] Wrong default value for entrypoint flags in job_submit.go
1 participant
0