kubeflow / sdk / 29604168366

84%

Build Type

push

github

Committed by web-flow

Commit Message

fix(trainer): validate polling_interval in wait_for_job_status (#554)

* fix(trainer): validate polling_interval in wait_for_job_status

Adds a ValueError guard in TrainerClient.wait_for_job_status() to reject non-positive polling_interval values before delegating to the backend. Previously this could cause a CPU busy-loop (polling_interval=0) or a cryptic stdlib error (negative values). Fixes #550

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* Potential fix for pull request finding add proper messages

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* test(trainer): update polling_interval test assertions to match new error messages

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* refactor(common): extract polling_interval/timeout validation to shared utils

Move polling_interval and timeout validation into a shared
common_utils.validate_wait_intervals() function, used by both
TrainerClient and OptimizerClient. Also renamed test function per
review feedback and fixed missing newline at end of test file.

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* Fix the formatting

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* fix: add missing newline at end of files

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* refactor(common): rename validate_wait_intervals to validate_wait_for_job_status

Per review feedback from @andreyvelich and @Goku2099, rename the shared
validation helper for clarity.

Signed-off-by: HarshPopat23 <musichk61@gmail.com>

* test(common): refactor validate_wait_for_job_status test to use TestCase pattern

Per review feedback from @andreyvelich:
- Refactor kubeflow/common/utils_test.py to use the repo's TestCase
  dataclass pattern for parametrized tests
- Remove redundant test_wait_for_job_status from trainer_client_test.py
  since validation logic is already covered in common/utils_test.py

Signed-off-by: HarshPopa... (continued)

Coverage Stats

21 of 23 new or added lines in 4 files covered. (91.3%)

3 existing lines in 2 files now uncovered.

5407 of 6428 relevant lines covered (84.12%)

1.68 hits per line