fix(trainer): changed ValueError to raise RuntimeError in localprocess and container in backends #433 by aashvgit · Pull Request #469 · kubeflow/sdk

aashvgit · 2026-04-21T18:05:17Z

What this PR does / why we need it:

get_job, get_job_logs, wait_for_job_status, delete_job in LocalProcess
backend now raise RuntimeError for job-not-found
_get_job_containers and __get_trainjob_from_containers in Container
backend now raise RuntimeError for job/network/runtime not found
Keep ValueError for input validation (polling_interval > timeout)

Fixes #
#433

github-actions · 2026-04-21T18:05:28Z

🎉 Welcome to the Kubeflow SDK! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Slack: Join our #kubeflow-ml-experience and #kubeflow-trainer Slack channels
Meetings: Attend the Kubeflow SDK and ML Experience bi-weekly meetings

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

google-oss-prow · 2026-04-21T18:05:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kramaranya for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Aligns LocalProcess and Container backends with the documented TrainerClient error contract by raising RuntimeError (instead of ValueError) for operational “not found” conditions such as missing jobs, containers, networks, or runtimes.

Changes:

LocalProcess backend: switch job-not-found and runtime-not-found errors to RuntimeError while keeping ValueError for input validation (e.g., polling_interval > timeout).
Container backend: switch job/container/network/runtime lookup failures to RuntimeError and update related docstrings.
Update unit tests to assert RuntimeError for these not-found scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
`kubeflow/trainer/backends/localprocess/backend.py`	Raises `RuntimeError` for runtime/job not found to match the public API contract.
`kubeflow/trainer/backends/localprocess/backend_test.py`	Updates expectations to `RuntimeError` for missing runtime/job cases.
`kubeflow/trainer/backends/container/backend.py`	Raises `RuntimeError` for missing containers/network/runtime during job reconstruction and lookup.
`kubeflow/trainer/backends/container/backend_test.py`	Updates expectations to `RuntimeError` for missing job cases.

kramaranya

Minor formatting issue found — see inline comment.

kramaranya

Thank you @aashvgit!

kramaranya · 2026-04-30T12:44:23Z

        local_runtime = next((rt for rt in local_runtimes if rt.name == runtime.name), None)
        if not local_runtime:
-            raise ValueError(f"Runtime '{runtime.name}' not found.")
+            raise   RuntimeError(f"Runtime '{runtime.name}' not found.")


Suggested change

raise RuntimeError(f"Runtime '{runtime.name}' not found.")

raise RuntimeError(f"Runtime '{runtime.name}' not found.")

cc @aashvgit

aashvgit · 2026-04-30T13:34:02Z

Thanks @kramaranya for the review I've fixed the line now

kramaranya · 2026-04-30T13:35:44Z

Could you rebase the changes and sign your commits please?

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot · 2026-05-01T18:56:34Z

            config={"name": BASIC_TRAIN_JOB_NAME, "timeout": 10, "polling_interval": -1},
            expected_error=ValueError,
+            config={"job_name": "nonexistent-job"},
+            expected_error=RuntimeError,
        ),


This TestCase currently passes config and expected_error twice, which is a Python syntax error and will prevent the test module from importing; split the job-not-found assertion into its own TestCase entry (using config={"name": "nonexistent-job"} and expected_error=RuntimeError, since wait_for_job_status(**config) expects name).

@aashvgit could you address it please?

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

        if not containers:
-            raise ValueError(f"No TrainJob with name {name}")
+            raise RuntimeError(f"No TrainJob with name {name}")


        network_id = containers[0]["labels"].get(f"{self.label_prefix}/network-id")
        if not network_id:
-            raise ValueError(f"TrainJob {job_name} is missing network metadata")
+            raise RuntimeError(f"TrainJob {job_name} is missing network metadata")

        network_info = self._adapter.get_network(network_id)
        if not network_info:
-            raise ValueError(f"TrainJob {job_name} network not found")
+            raise RuntimeError(f"TrainJob {job_name} network not found")


        try:
            job_runtime = self.get_runtime(runtime_name) if runtime_name else None
        except Exception as e:
-            raise ValueError(f"Runtime {runtime_name} not found for job {job_name}") from e
+            raise RuntimeError(f"Runtime {runtime_name} not found for job {job_name}") from e

        if not job_runtime:
-            raise ValueError(f"Runtime {runtime_name} not found for job {job_name}")
+            raise RuntimeError(f"Runtime {runtime_name} not found for job {job_name}")


…s and container backends Signed-off-by: aashvgit <167199295+aashvgit@users.noreply.github.com>

google-oss-prow Bot added the do-not-merge/work-in-progress label Apr 21, 2026

google-oss-prow Bot requested review from Electronic-Waste, andreyvelich and szaher April 21, 2026 18:05

google-oss-prow Bot added the size/M label Apr 21, 2026

aashvgit marked this pull request as ready for review April 21, 2026 18:18

Copilot AI review requested due to automatic review settings April 21, 2026 18:18

google-oss-prow Bot removed the do-not-merge/work-in-progress label Apr 21, 2026

google-oss-prow Bot requested review from astefanutti and kramaranya April 21, 2026 18:18

Copilot started reviewing on behalf of aashvgit April 21, 2026 18:18 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

kramaranya reviewed Apr 30, 2026

View reviewed changes

aashvgit force-pushed the fix/433-valueerror-to-runtimeerror branch from fb06144 to 811cfb9 Compare April 30, 2026 14:40

aashvgit requested review from Copilot and kramaranya May 1, 2026 18:53

Copilot AI reviewed May 1, 2026

View reviewed changes

aashvgit force-pushed the fix/433-valueerror-to-runtimeerror branch from 811cfb9 to fb1c808 Compare May 5, 2026 09:52

aashvgit requested a review from Copilot May 5, 2026 09:52

Copilot started reviewing on behalf of aashvgit May 5, 2026 09:53 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

aashvgit force-pushed the fix/433-valueerror-to-runtimeerror branch from fb1c808 to 41eba7d Compare May 5, 2026 10:01

fix(trainer): changed ValueError to raise RuntimeError in localproces…

912039f

…s and container backends Signed-off-by: aashvgit <167199295+aashvgit@users.noreply.github.com>

aashvgit force-pushed the fix/433-valueerror-to-runtimeerror branch from 41eba7d to 912039f Compare May 8, 2026 18:38

	raise RuntimeError(f"Runtime '{runtime.name}' not found.")
	raise RuntimeError(f"Runtime '{runtime.name}' not found.")

Conversation

aashvgit commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 21, 2026

Uh oh!

google-oss-prow Bot commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

kramaranya left a comment

Choose a reason for hiding this comment

Uh oh!

kramaranya left a comment

Choose a reason for hiding this comment

Uh oh!

kramaranya Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

kramaranya May 5, 2026

Choose a reason for hiding this comment

Uh oh!

aashvgit commented Apr 30, 2026

Uh oh!

kramaranya commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

kramaranya May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants