[train] Provide structured logging env var #52952
Conversation
Signed-off-by: Timothy Seah <[email protected]>
Signed-off-by: Timothy Seah <[email protected]>
TimothySeah
requested review from
hongpeng-guo,
justinvyu,
matthewdeng,
raulchen and
woshiyyya
as code owners
python/ray/train/v2/_internal/execution/controller/controller.py
Outdated
Show resolved
Hide resolved
| @pytest.mark.parametrize("context", [get_dummy_run_context()]) | ||
| def test_controller_sys_not_logged_to_file(controller_logging, context): |
There was a problem hiding this comment.
can you make this a @pytest.mark.parametrize("configure_logging", [True, False])?
And also, can you do a driveby change to convert the current "context" parameter to just a regular fixture since it's always a single value.
There was a problem hiding this comment.
Converted context - lmk if this is what you had in mind since I'm new to pytest.
I initially tried can you make this a @pytest.mark.parametrize("configure_logging", [True, False]) but decided against it because the branching logic (toggling whether we call configure_controller_logger and then assert contents vs assert raises) is even more lines of code. Lmk what you think.
There was a problem hiding this comment.
I think I prefer these 2 tests to be packaged in the same method even if it's more lines of code, to make it clear that the 2nd case is the omission of the call to configure_x_logging. But I'm not going to block merge on this nit
| @pytest.mark.parametrize("context", [get_dummy_run_context()]) | ||
| def test_controller_sys_not_logged_to_file(controller_logging, context): |
There was a problem hiding this comment.
I think I prefer these 2 tests to be packaged in the same method even if it's more lines of code, to make it clear that the 2nd case is the omission of the call to configure_x_logging. But I'm not going to block merge on this nit
Provide env vars to enable/disable structured logging on controller and worker processes.
Verified on ray train v2 on Anyscale with the pytorch example: https://docs.ray.io/en/latest/train/getting-started-pytorch.html.
With no environment variables set the controller and worker logs look like this
with the environment variables set the controller and worker logs look like this (note how there are no controller logs at all)
