Skip to content

Conversation

@njhill
Copy link
Member

@njhill njhill commented Nov 17, 2025

Addressing this review comment from #28768.

Signed-off-by: Nick Hill <nhill@redhat.com>
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 renames the ec_producer field to is_ec_producer in vllm/v1/engine/core.py. This change improves code clarity and follows Python's naming conventions for boolean variables.

However, the refactoring appears to be incomplete. I've noticed that vllm/v1/executor/ray_executor.py still uses the old ec_producer name. To ensure consistency across the codebase and prevent potential confusion or bugs in the future, I recommend renaming self.ec_producer to self.is_ec_producer in vllm/v1/executor/ray_executor.py as well. Specifically, the definition at lines 103-106 and its usage at line 404 should be updated.

@WoosukKwon WoosukKwon added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 17, 2025
@WoosukKwon WoosukKwon enabled auto-merge (squash) November 17, 2025 23:36
@WoosukKwon WoosukKwon merged commit da8dadf into vllm-project:main Nov 18, 2025
43 checks passed
@njhill njhill deleted the rename-core-field branch November 18, 2025 17:28
Victor49152 pushed a commit to Victor49152/vllm that referenced this pull request Nov 20, 2025
…28884)

Signed-off-by: Nick Hill <nhill@redhat.com>
bhagyashrigai pushed a commit to odh-on-pz/vllm-upstream that referenced this pull request Nov 20, 2025
…28884)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: Bhagyashri <Bhagyashri.Gaikwad2@ibm.com>
bigPYJ1151 pushed a commit that referenced this pull request Nov 25, 2025
Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: jiang1.li <jiang1.li@intel.com>
bringlein pushed a commit to bringlein/vllm that referenced this pull request Nov 26, 2025
…28884)

Signed-off-by: Nick Hill <nhill@redhat.com>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
…28884)

Signed-off-by: Nick Hill <nhill@redhat.com>
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.

2 participants