Skip to content

Conversation

edoakes
Copy link
Collaborator

@edoakes edoakes commented Jun 13, 2025

Deflakes the test by ensuring that results come in out of order for the imap_unordered tests instead of relying on random time.sleeps.

In the process of rewriting the tests, I discovered a bug in the implementation. We weren't re-fetching available object refs from the queue once we had gotten at least one, which caused the tests to hang. Added a timeout to the ray.wait call so we continually check for new object refs to add to the batch.

edoakes added 2 commits June 13, 2025 09:34
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes added the go add ONLY when ready to merge, run all tests label Jun 13, 2025
@edoakes edoakes requested a review from a team June 13, 2025 14:37
edoakes added 2 commits June 13, 2025 09:38
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Copy link
Contributor

@codope codope left a comment

Choose a reason for hiding this comment

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

LGTM! One minor clarification comment.

@codope codope self-assigned this Jun 16, 2025
@edoakes edoakes enabled auto-merge (squash) June 16, 2025 16:14
@edoakes edoakes merged commit 8f8013f into ray-project:master Jun 16, 2025
6 checks passed
@edoakes edoakes mentioned this pull request Jun 16, 2025
elliot-barn pushed a commit that referenced this pull request Jun 18, 2025
Deflakes the test by ensuring that results come in out of order for the
`imap_unordered` tests instead of relying on random `time.sleep`s.

In the process of rewriting the tests, I discovered a bug in the
implementation. We weren't re-fetching available object refs from the
queue once we had gotten at least one, which caused the tests to hang.
Added a timeout to the `ray.wait` call so we continually check for new
object refs to add to the batch.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
edoakes added a commit that referenced this pull request Jun 25, 2025
Meant to add comment in #53802,
but there was a race condition with auto merge.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
minerharry pushed a commit to minerharry/ray that referenced this pull request Jun 27, 2025
Deflakes the test by ensuring that results come in out of order for the
`imap_unordered` tests instead of relying on random `time.sleep`s.

In the process of rewriting the tests, I discovered a bug in the
implementation. We weren't re-fetching available object refs from the
queue once we had gotten at least one, which caused the tests to hang.
Added a timeout to the `ray.wait` call so we continually check for new
object refs to add to the batch.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
minerharry pushed a commit to minerharry/ray that referenced this pull request Jun 27, 2025
Meant to add comment in ray-project#53802,
but there was a race condition with auto merge.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
Deflakes the test by ensuring that results come in out of order for the
`imap_unordered` tests instead of relying on random `time.sleep`s.

In the process of rewriting the tests, I discovered a bug in the
implementation. We weren't re-fetching available object refs from the
queue once we had gotten at least one, which caused the tests to hang.
Added a timeout to the `ray.wait` call so we continually check for new
object refs to add to the batch.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
Meant to add comment in #53802,
but there was a race condition with auto merge.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants