Skip to content

Conversation

@zhenwei-intel
Copy link
Contributor

The recent update of NIXL's whl filename (nixl_cu12-0.6.1-cp312-cp312-linux_x86_64.whl) caused find_nixl_wheel_in_cache to fail.

Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
@mergify mergify bot added the kv-connector label Oct 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

The pull request updates the search pattern for finding the NIXL wheel file to accommodate a change in the wheel's filename format. While the change fixes the immediate issue, the new glob pattern is a bit too broad and could potentially match unrelated wheel files. I've suggested a more specific approach using two glob patterns to make the search more robust, ensuring it correctly identifies both old and new filename formats without accidentally picking up other packages.

Comment on lines +40 to 41
search_pattern = os.path.join(cache_dir, "nixl*.whl")
wheels = glob.glob(search_pattern)
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The glob pattern nixl*.whl is a bit too broad. It would match any file starting with nixl and ending with .whl, such as a hypothetical nixltools-1.0.whl. This could lead to selecting the wrong wheel file if multiple such files exist in the cache and the sort order is not favorable.

To make the search more robust, it's better to use two more specific glob patterns: one for the old format (nixl-*.whl) and one for the new format (nixl_*-*.whl). This ensures we only match wheels for the nixl package, accommodating both naming conventions while being less likely to match unrelated packages.

Suggested change
search_pattern = os.path.join(cache_dir, "nixl*.whl")
wheels = glob.glob(search_pattern)
wheels = glob.glob(os.path.join(cache_dir, "nixl-*.whl"))
wheels.extend(glob.glob(os.path.join(cache_dir, "nixl_*-*.whl")))

@jikunshang jikunshang added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 28, 2025
@jikunshang jikunshang merged commit d2c33c3 into vllm-project:main Oct 29, 2025
21 checks passed
bhagyashrigai pushed a commit to odh-on-pz/vllm-upstream that referenced this pull request Oct 29, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
Signed-off-by: Bhagyashri <Bhagyashri.Gaikwad2@ibm.com>
MatthewBonanni pushed a commit to MatthewBonanni/vllm that referenced this pull request Oct 30, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
@ovidiusm
Copy link

In the next NIXL release, the package will be split into nixl and nixl-cu12 / nixl-cu13.

You are installing only nixl-cu12 but missing nixl. That one can be found in the build tree in build/src/bindings/python/nixl-meta/nixl-*.whl

This is still in the works so it may make sense to wait a week or so for things to stabilize

@zhenwei-intel
Copy link
Contributor Author

In the next NIXL release, the package will be split into nixl and nixl-cu12 / nixl-cu13.

You are installing only nixl-cu12 but missing nixl. That one can be found in the build tree in build/src/bindings/python/nixl-meta/nixl-*.whl

This is still in the works so it may make sense to wait a week or so for things to stabilize

Thanks, I'll keep tracking this.​

yma11 pushed a commit to yma11/vllm that referenced this pull request Nov 5, 2025
…ect#389)

Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
ilmarkov pushed a commit to neuralmagic/vllm that referenced this pull request Nov 7, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
ZhengHongming888 pushed a commit to ZhengHongming888/vllm that referenced this pull request Nov 8, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kv-connector 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.

3 participants