Skip to content

Conversation

matthewdeng
Copy link
Contributor

Description

Bump from small to medium due to timeouts happening specifically in py3.12 tests.

@matthewdeng matthewdeng requested a review from a team as a code owner October 18, 2025 02:54
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses test timeouts for test_torch_trainer on Python 3.12 by increasing the test size from small to medium. This is a valid approach. However, I've suggested a more precise solution using Bazel's select functionality to apply the larger timeout only when building with Python 3.12. This would avoid increasing the timeout for other Python versions, contributing to better CI efficiency. See my comment for an example implementation.

py_test(
name = "test_torch_trainer",
size = "small",
size = "medium",
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

A more targeted fix for the Python 3.12 timeouts would be to use Bazel's select statement. This would apply the medium size only for the Python 3.12 build, while other versions would continue to use the small size, which can help keep CI runs fast.

Here's an example of how you could implement this:

First, define a config_setting (if one doesn't already exist for your toolchain):

# In a relevant BUILD file (e.g., the root BUILD file)
config_setting(
    name = "is_py312",
    flag_values = {
        "@bazel_tools//tools/python:python_version": "PY3.12",
    },
)

Then, use select in this py_test definition:

py_test(
    name = "test_torch_trainer",
    size = select({
        "//path/to:is_py312": "medium",
        "//conditions:default": "small",
    }),
    ...
)

@matthewdeng matthewdeng enabled auto-merge (squash) October 18, 2025 04:36
@github-actions github-actions bot disabled auto-merge October 18, 2025 04:36
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Oct 18, 2025
@ray-gardener ray-gardener bot added the train Ray Train Related Issue label Oct 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests train Ray Train Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants