Skip to content

[XPU] add missing dependency tblib for XPU CI#24639

Merged
jikunshang merged 3 commits intovllm-project:mainfrom
faaany:add_tblib
Sep 11, 2025
Merged

[XPU] add missing dependency tblib for XPU CI#24639
jikunshang merged 3 commits intovllm-project:mainfrom
faaany:add_tblib

Conversation

@faaany
Copy link
Copy Markdown
Contributor

@faaany faaany commented Sep 11, 2025

Purpose

This PR adds the missing dependency introduced in #23795 to fix the following failure in XPU CI:

image

Test Result

All tests passed.

faaany and others added 2 commits September 11, 2025 00:15
Signed-off-by: Fanli Lin <fanli.lin@intel.com>
@mergify mergify bot added the ci/build label Sep 11, 2025
Copy link
Copy Markdown
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 addresses a CI failure on XPU by adding the missing tblib dependency. While the change is effective, it installs the dependency directly within the test script. My review suggests moving this dependency into a dedicated requirements file to improve maintainability and build consistency. This is a more robust long-term solution for dependency management.

bash -c '
set -e
echo $ZE_AFFINITY_MASK
pip install tblib==3.1.0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

For better dependency management and to ensure a consistent build environment, it's recommended to add this dependency to a requirements file instead of installing it directly in the CI script. This helps in centralizing dependency management and leverages Docker's layer caching, potentially speeding up the CI process.

Please consider adding tblib==3.1.0 to the appropriate requirements file (e.g., requirements/xpu.txt as used in docker/Dockerfile.xpu) and removing this line from the script.

Copy link
Copy Markdown
Contributor Author

@faaany faaany Sep 11, 2025

Choose a reason for hiding this comment

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

tblib is a test-only dependency. So adding to "xpu.txt" might not be appropriate. But adding an extra "xpu-test.txt" file is also unnecessary for just one dependency. So for now, I think install it directly in the CI script is the most straightforward way.

@faaany
Copy link
Copy Markdown
Contributor Author

faaany commented Sep 11, 2025

cc @jikunshang @yma11 @rogerxfeng8

Copy link
Copy Markdown
Collaborator

@jikunshang jikunshang left a comment

Choose a reason for hiding this comment

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

thanks for fixing!

@jikunshang jikunshang added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 11, 2025
@jikunshang jikunshang enabled auto-merge (squash) September 11, 2025 08:51
@jikunshang jikunshang merged commit 0cd72a7 into vllm-project:main Sep 11, 2025
16 checks passed
skyloevil pushed a commit to skyloevil/vllm that referenced this pull request Sep 13, 2025
Signed-off-by: Fanli Lin <fanli.lin@intel.com>
dsxsteven pushed a commit to dsxsteven/vllm_splitPR that referenced this pull request Sep 15, 2025
Signed-off-by: Fanli Lin <fanli.lin@intel.com>
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: Fanli Lin <fanli.lin@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants