Skip to content

Conversation

seongsukwon-moreh
Copy link
Contributor

This pull request makes a small change to the job retrieval logic in sky/utils/resource_checker.py. The function get_all_managed_jobs() now returns only the filtered jobs instead of the full output from managed_jobs_core.queue(), ensuring that only relevant job data is provided.

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
  • Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
  • Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch @seongsukwon-moreh! Thanks a lot for the fix.

cc'ing @DanielZhangQD, we should check why the previous PR fail to catch this issue, e.g., the mypy check should be able to catch this?

@DanielZhangQD
Copy link
Collaborator

Hi @seongsukwon-moreh,

Thanks for catching this issue!

For why this type issue is not caught by our code linter, I checked and found that it's because we do not set the return type of the functions, after I add the return type to the functions, our format.sh reports the following error:

sky/utils/resource_checker.py:279: error: Incompatible return value type (got "Tuple[List[Dict[str, Any]], int, Dict[str, int], int]", expected "List[Dict[str, Any]]")  [return-value]

cc @Michaelvll

return managed_jobs_core.queue(refresh=False,
skip_finished=True,
all_users=True)
filtered_jobs, _, _, _ = managed_jobs_core.queue(refresh=False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @seongsukwon-moreh, could you please help add return type List[Dict[str, Any]] for the get_all_clusters and get_all_managed_jobs functions, so that any mismatch changes can be caught later?
Thanks!

Copy link
Collaborator

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks! @seongsukwon-moreh

@seongsukwon-moreh seongsukwon-moreh marked this pull request as ready for review September 3, 2025 07:26
@Michaelvll Michaelvll enabled auto-merge (squash) September 3, 2025 07:27
@Michaelvll Michaelvll merged commit cf79038 into skypilot-org:master Sep 3, 2025
16 checks passed
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.

3 participants