[Utils] [CORE-658] ray.util.Queue Empty and Full exceptions extend queue.Empty and Full #50261
Conversation
to match what multiprocessing.Queue does to simplify client exception handling Signed-off-by: Ibrahim Rabbani <[email protected]>
israbbani
requested review from
hongpeng-guo,
justinvyu,
matthewdeng,
raulchen,
woshiyyya,
hongchaodeng and
a team
as code owners
|
|
||
| import ray | ||
| from ray.util.annotations import PublicAPI | ||
|
|
||
|
|
||
| @PublicAPI(stability="beta") |
There was a problem hiding this comment.
As it is marked as beta, I'm wondering if we should follow the deprecation period mentioned here: https://docs.ray.io/en/latest/ray-contribute/stability.html#beta
cc: @jjyao
There was a problem hiding this comment.
Makes sense. I think after Jiajun's comment, I'm going to make the change backwards compatible.
|
|
||
| import ray | ||
| from ray.util.annotations import PublicAPI | ||
|
|
||
|
|
||
| @PublicAPI(stability="beta") | ||
| class Empty(Exception): |
There was a problem hiding this comment.
For backward compatibility, can we make the current Empty and Full the subclass of python Empty and Full?
There was a problem hiding this comment.
Made the change backwards compatible. Would any of the integration tests have caught this backwards incompatible change?
There was a problem hiding this comment.
Probably we can add a test in python/ray/tests/test_queue.py to validate that the queue will through the expected type of Empty object in the empty scenario.
There was a problem hiding this comment.
Unless I'm misunderstanding, python/ray/tests/test_queue.py already checks for both Empty and Full exceptions.
I was wondering if there's an API-level spec file or protocol definition that ensures backwards compatibility and disallows incompatible changes.
There was a problem hiding this comment.
Makes sense for the test part.
For the API spec, I think the annotation is the only thing that we have for now. cc: @jjyao in case you have more context.
compatibility Signed-off-by: Ibrahim Rabbani <[email protected]>
israbbani
changed the title
israbbani
changed the title
|
Thanks for doing this!! 🎉 |
Simplify ray.util.Queue usage and exception handling by extending queue.Empty and queue.Full.
Closes #50154