Skip to content

Conversation

@LucasWilkinson
Copy link
Collaborator

@LucasWilkinson LucasWilkinson commented Nov 28, 2025

#28579 broke DBO but was hidden by existing test failures on the nightly (#28893, with those fixed this now showed up)

Test Plan:

tests/v1/distributed/test_dbo.py

Fails on Main, Passes on this PR

NOTE: using padded_second_ubatch_slice instead of ubatch_slices[1].request_slice for requests was a bug that pre-dated #28579 but was just exacerbated by it

@mergify mergify bot added the v1 label Nov 28, 2025
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 correctly fixes a TypeError in the DBO logic by adding a missing return statement. The change is correct and resolves the issue. I have added one comment with suggestions for further improving code quality and maintainability in the affected function.

@LucasWilkinson LucasWilkinson added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 28, 2025
@LucasWilkinson
Copy link
Collaborator Author

NVM its flaky; holding off for now

Signed-off-by: Lucas Wilkinson <[email protected]>
Signed-off-by: Lucas Wilkinson <[email protected]>
Signed-off-by: Lucas Wilkinson <[email protected]>
Signed-off-by: Lucas Wilkinson <[email protected]>
Signed-off-by: Lucas Wilkinson <[email protected]>
@vllm-bot vllm-bot merged commit e23f665 into vllm-project:main Nov 29, 2025
49 of 51 checks passed
kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
amd-hhashemi pushed a commit to amd-hhashemi/vllm that referenced this pull request Dec 2, 2025
minosfuture added a commit to minosfuture/vllm that referenced this pull request Dec 4, 2025
charlotte12l pushed a commit to charlotte12l/vllm that referenced this pull request Dec 5, 2025
Zhathw pushed a commit to Zhathw/vllm that referenced this pull request Dec 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants