From 642e76018fc8fe497d6e1fbfe7777802731ac347 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Thu, 15 Dec 2022 20:34:38 +0000 Subject: [PATCH 1/9] timeout 0 warnings and doc Signed-off-by: rickyyx --- python/ray/_private/worker.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 8b8f63b8f9c0..50049ff99000 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2273,7 +2273,9 @@ def get( object_refs: Object ref of the object to get or a list of object refs to get. timeout (Optional[float]): The maximum amount of time in seconds to - wait before returning. + wait before returning. Set this to None will block until the + corresponding object becomes available. Set this to 0 will return + the object immediately if it's available, else raise GetTimeoutError. Returns: A Python object or a list of Python objects. @@ -2284,6 +2286,18 @@ def get( Exception: An exception is raised if the task that created the object or that created one of the objects raised an exception. """ + if timeout == 0: + import warnings + + warnings.warn( + ( + "Please use timeout=None if you expect ray.get() to block. " + "Setting timeout=0 in future ray releases will raise GetTimeoutError " + "if the objects references are not available." + ), + SyntaxWarning, + ) + worker = global_worker worker.check_connected() From 99502f428f3fb7c08374a4a9bff9f65297dc2aee Mon Sep 17 00:00:00 2001 From: rickyyx Date: Thu, 15 Dec 2022 20:39:42 +0000 Subject: [PATCH 2/9] doc Signed-off-by: rickyyx --- python/ray/_private/worker.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 50049ff99000..c6845bdfe960 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2274,8 +2274,10 @@ def get( to get. timeout (Optional[float]): The maximum amount of time in seconds to wait before returning. Set this to None will block until the - corresponding object becomes available. Set this to 0 will return - the object immediately if it's available, else raise GetTimeoutError. + corresponding object becomes available. + + WARNING: In future ray releases, set ``timeout=0`` will return the object + immediately if it's available, else raise GetTimeoutError. Returns: A Python object or a list of Python objects. From cbf05659744f7de52c62dde2b2a87c216c1e1620 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Thu, 15 Dec 2022 20:53:35 +0000 Subject: [PATCH 3/9] test Signed-off-by: rickyyx --- python/ray/_private/worker.py | 2 +- python/ray/tests/test_basic_2.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index c6845bdfe960..21867e85ba58 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2297,7 +2297,7 @@ def get( "Setting timeout=0 in future ray releases will raise GetTimeoutError " "if the objects references are not available." ), - SyntaxWarning, + UserWarning, ) worker = global_worker diff --git a/python/ray/tests/test_basic_2.py b/python/ray/tests/test_basic_2.py index b83280fcecb6..6323b9b0bd46 100644 --- a/python/ray/tests/test_basic_2.py +++ b/python/ray/tests/test_basic_2.py @@ -390,6 +390,11 @@ def test_get_with_timeout(ray_start_regular_shared): ray.get(result_id, timeout=30) assert time.time() - start < 30 + # Check that ray.get(timeout=0) raises warnings on change of behavior. + # Removed when https://github.com/ray-project/ray/issues/28465 is resolved. + with pytest.warns(UserWarning): + ray.get(signal.wait.remote(should_wait=False), timeout=0) + # https://github.com/ray-project/ray/issues/6329 def test_call_actors_indirect_through_tasks(ray_start_regular_shared): From ecdbdc21bf9a29dbd4927393821c300890b2f056 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 19:26:01 +0000 Subject: [PATCH 4/9] comments Signed-off-by: rickyyx --- python/ray/_private/worker.py | 14 +++++++++----- python/ray/tests/test_basic_2.py | 10 +++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 21867e85ba58..b1596733d0fb 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2275,9 +2275,11 @@ def get( timeout (Optional[float]): The maximum amount of time in seconds to wait before returning. Set this to None will block until the corresponding object becomes available. - - WARNING: In future ray releases, set ``timeout=0`` will return the object - immediately if it's available, else raise GetTimeoutError. + WARNING: In future ray releases ``timeout=0`` will return the object + immediately if it's available, else raise GetTimeoutError in accordance with + the above docstring. The current behavior of blocking until objects become + available of ``timeout=0`` is considered to be a bug, see + https://github.com/ray-project/ray/issues/28465. Returns: A Python object or a list of Python objects. @@ -2288,14 +2290,16 @@ def get( Exception: An exception is raised if the task that created the object or that created one of the objects raised an exception. """ - if timeout == 0: + if timeout == 0 and os.environ.get("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "1") == "1": import warnings warnings.warn( ( "Please use timeout=None if you expect ray.get() to block. " "Setting timeout=0 in future ray releases will raise GetTimeoutError " - "if the objects references are not available." + "if the objects references are not available. " + "You could suppress this warning by setting " + "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0" ), UserWarning, ) diff --git a/python/ray/tests/test_basic_2.py b/python/ray/tests/test_basic_2.py index 6323b9b0bd46..aa2fe436dff6 100644 --- a/python/ray/tests/test_basic_2.py +++ b/python/ray/tests/test_basic_2.py @@ -365,7 +365,7 @@ def test_get_multiple(ray_start_regular_shared): assert results == indices -def test_get_with_timeout(ray_start_regular_shared): +def test_get_with_timeout(ray_start_regular_shared, monkeypatch): SignalActor = create_remote_signal_actor(ray) signal = SignalActor.remote() @@ -395,6 +395,14 @@ def test_get_with_timeout(ray_start_regular_shared): with pytest.warns(UserWarning): ray.get(signal.wait.remote(should_wait=False), timeout=0) + with monkeypatch.context() as m: + m.setenv("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "0") + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("error") + ray.get(signal.wait.remote(should_wait=False), timeout=0) + # https://github.com/ray-project/ray/issues/6329 def test_call_actors_indirect_through_tasks(ray_start_regular_shared): From af876a62cfc06f389c47bda3679c2a8fa778442c Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 19:45:42 +0000 Subject: [PATCH 5/9] for next pr Signed-off-by: rickyyx --- python/ray/_private/worker.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index b1596733d0fb..14da06b3fa8e 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2290,19 +2290,20 @@ def get( Exception: An exception is raised if the task that created the object or that created one of the objects raised an exception. """ - if timeout == 0 and os.environ.get("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "1") == "1": - import warnings + if timeout == 0: + if os.environ.get("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "1") == "1": + import warnings - warnings.warn( - ( - "Please use timeout=None if you expect ray.get() to block. " - "Setting timeout=0 in future ray releases will raise GetTimeoutError " - "if the objects references are not available. " - "You could suppress this warning by setting " - "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0" - ), - UserWarning, - ) + warnings.warn( + ( + "Please use timeout=None if you expect ray.get() to block. " + "Setting timeout=0 in future ray releases will raise " + "GetTimeoutError if the objects references are not available. " + "You could suppress this warning by setting " + "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0" + ), + UserWarning, + ) worker = global_worker worker.check_connected() From a2f8401f0ee6b8e3b6141f367dd8379364d4fb20 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 19:49:10 +0000 Subject: [PATCH 6/9] . Signed-off-by: rickyyx --- python/ray/_private/worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 14da06b3fa8e..6da5220190d4 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -2300,7 +2300,7 @@ def get( "Setting timeout=0 in future ray releases will raise " "GetTimeoutError if the objects references are not available. " "You could suppress this warning by setting " - "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0" + "RAY_WARN_RAY_GET_TIMEOUT_ZERO=0." ), UserWarning, ) From 556a123f397074dda77374ad24663c3d66871891 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 21:31:01 +0000 Subject: [PATCH 7/9] move test to advanced Signed-off-by: rickyyx --- python/ray/tests/test_advanced.py | 15 +++++++++++++++ python/ray/tests/test_basic_2.py | 13 ------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/python/ray/tests/test_advanced.py b/python/ray/tests/test_advanced.py index 38fe09eca8d6..d4574eb51e16 100644 --- a/python/ray/tests/test_advanced.py +++ b/python/ray/tests/test_advanced.py @@ -383,6 +383,21 @@ def test_illegal_api_calls(ray_start_regular): ray.get(3) +def test_ray_get_timeout_zero(monkeypatch): + # Check that ray.get(timeout=0) raises warnings on change of behavior. + # Removed when https://github.com/ray-project/ray/issues/28465 is resolved. + with pytest.warns(UserWarning): + ray.get(ray.put(1), timeout=0) + + with monkeypatch.context() as m: + m.setenv("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "0") + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("error") + ray.get(ray.put(1), timeout=0) + + if __name__ == "__main__": import pytest diff --git a/python/ray/tests/test_basic_2.py b/python/ray/tests/test_basic_2.py index aa2fe436dff6..01b0e0d50799 100644 --- a/python/ray/tests/test_basic_2.py +++ b/python/ray/tests/test_basic_2.py @@ -390,19 +390,6 @@ def test_get_with_timeout(ray_start_regular_shared, monkeypatch): ray.get(result_id, timeout=30) assert time.time() - start < 30 - # Check that ray.get(timeout=0) raises warnings on change of behavior. - # Removed when https://github.com/ray-project/ray/issues/28465 is resolved. - with pytest.warns(UserWarning): - ray.get(signal.wait.remote(should_wait=False), timeout=0) - - with monkeypatch.context() as m: - m.setenv("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "0") - import warnings - - with warnings.catch_warnings(): - warnings.simplefilter("error") - ray.get(signal.wait.remote(should_wait=False), timeout=0) - # https://github.com/ray-project/ray/issues/6329 def test_call_actors_indirect_through_tasks(ray_start_regular_shared): From 2b59bdb498e66352ba313e942a2483762eb2fbfb Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 21:35:31 +0000 Subject: [PATCH 8/9] removed import Signed-off-by: rickyyx --- python/ray/tests/test_basic_2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/tests/test_basic_2.py b/python/ray/tests/test_basic_2.py index 01b0e0d50799..b83280fcecb6 100644 --- a/python/ray/tests/test_basic_2.py +++ b/python/ray/tests/test_basic_2.py @@ -365,7 +365,7 @@ def test_get_multiple(ray_start_regular_shared): assert results == indices -def test_get_with_timeout(ray_start_regular_shared, monkeypatch): +def test_get_with_timeout(ray_start_regular_shared): SignalActor = create_remote_signal_actor(ray) signal = SignalActor.remote() From c25edefbde7ee928c3cddd5f800bc4339beea098 Mon Sep 17 00:00:00 2001 From: rickyyx Date: Fri, 16 Dec 2022 23:11:21 +0000 Subject: [PATCH 9/9] move again Signed-off-by: rickyyx --- python/ray/tests/test_advanced.py | 15 --------------- python/ray/tests/test_advanced_2.py | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/python/ray/tests/test_advanced.py b/python/ray/tests/test_advanced.py index d4574eb51e16..38fe09eca8d6 100644 --- a/python/ray/tests/test_advanced.py +++ b/python/ray/tests/test_advanced.py @@ -383,21 +383,6 @@ def test_illegal_api_calls(ray_start_regular): ray.get(3) -def test_ray_get_timeout_zero(monkeypatch): - # Check that ray.get(timeout=0) raises warnings on change of behavior. - # Removed when https://github.com/ray-project/ray/issues/28465 is resolved. - with pytest.warns(UserWarning): - ray.get(ray.put(1), timeout=0) - - with monkeypatch.context() as m: - m.setenv("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "0") - import warnings - - with warnings.catch_warnings(): - warnings.simplefilter("error") - ray.get(ray.put(1), timeout=0) - - if __name__ == "__main__": import pytest diff --git a/python/ray/tests/test_advanced_2.py b/python/ray/tests/test_advanced_2.py index e1086c6e83f8..6234f7bacfe8 100644 --- a/python/ray/tests/test_advanced_2.py +++ b/python/ray/tests/test_advanced_2.py @@ -520,6 +520,21 @@ def test(): assert cluster_resources == {} +def test_ray_get_timeout_zero(monkeypatch): + # Check that ray.get(timeout=0) raises warnings on change of behavior. + # Removed when https://github.com/ray-project/ray/issues/28465 is resolved. + with pytest.warns(UserWarning): + ray.get(ray.put(1), timeout=0) + + with monkeypatch.context() as m: + m.setenv("RAY_WARN_RAY_GET_TIMEOUT_ZERO", "0") + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("error") + ray.get(ray.put(1), timeout=0) + + if __name__ == "__main__": if os.environ.get("PARALLEL_CI"): sys.exit(pytest.main(["-n", "auto", "--boxed", "-vs", __file__]))