[CI] Fix image merge bug#5197
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes a temporary workaround from the nightly test script that was pulling the latest changes from git. The change itself is just the removal of the upgrade_vllm_ascend_scr function. However, my review indicates that the call to this function within the main function was not removed, which will break the script. This is a critical issue that needs to be addressed by removing the dangling function call.
I am having trouble creating individual review comments. Click here to see my feedback.
tests/e2e/nightly/multi_node/scripts/run.sh (127-135)
The upgrade_vllm_ascend_scr function is being removed, which is correct according to the PR's goal. However, based on the provided file content, the call to this function at line 149 within the main function appears to have been left in. This will lead to a script failure with a 'command not found' error. Please ensure the call at line 149 is also deleted.
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
### What this PR does / why we need it? Some tiny bugfix for vllm-project#5175 Signed-off-by: wangli <wangli858794774@gmail.com>
### What this PR does / why we need it? Some tiny bugfix for vllm-project#5175 Signed-off-by: wangli <wangli858794774@gmail.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
### What this PR does / why we need it? Some tiny bugfix for vllm-project#5175 Signed-off-by: wangli <wangli858794774@gmail.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
What this PR does / why we need it?
Some tiny bugfix for #5175
How was this patch tested?