[Misc] code clean duplicate set_current_vllm_config in _set_vllm_config #22566
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly removes a redundant and ineffective call to set_current_vllm_config. The change is a good code cleanup.
While reviewing this function, I noticed a potential resource leak. The temporary file created on line 37 with tempfile.mkstemp() is not being cleaned up after use. This could lead to an accumulation of temporary files, potentially causing issues in CI environments.
I recommend addressing this in a follow-up pull request by using a context manager, for example tempfile.NamedTemporaryFile, to ensure the temporary file is automatically deleted. This is just a suggestion for future improvement and doesn't block this PR.
Signed-off-by: Andy Xie <andy.xning@gmail.com>
andyxning
force-pushed
the
code_clean_duplicate_set_current_vllm_config_in__set_vllm_config
branch
from
65f09de to
2078a06
Compare
github-actions
bot
added
the
ready
|
Seems ci failures are flakes and some error about entrypoint which also fails in other PRs. Can you help force merge of this. @DarkLight1337 |
andyxning
deleted the
code_clean_duplicate_set_current_vllm_config_in__set_vllm_config
branch
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com> Signed-off-by: jingyu <jingyu@omniml.ai>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com> Signed-off-by: Avery Yingyi Huang <yingyihuang2000@outlook.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com> Signed-off-by: Boyuan Feng <boyuan@meta.com>
…ig (vllm-project#22566) Signed-off-by: Andy Xie <andy.xning@gmail.com> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
set_current_vllm_configis duplicated.Test Plan
NA
Test Result
NA
(Optional) Documentation Update
NA