Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] [GHA] Introduce additional Python (3.9-3.12) API tests on Windows #27304

Open
wants to merge 38 commits into
base: master
Choose a base branch
from

Conversation

akashchi
Copy link
Contributor

Tickets:

  • 152705

@akashchi akashchi added the WIP work in progress label Oct 29, 2024
@akashchi akashchi added this to the 2024.5 milestone Oct 29, 2024
@github-actions github-actions bot added category: CI OpenVINO public CI github_actions Pull requests that update GitHub Actions code labels Oct 29, 2024
@akashchi akashchi removed the WIP work in progress label Nov 5, 2024
@akashchi akashchi marked this pull request as ready for review November 5, 2024 11:54
@akashchi akashchi requested a review from a team as a code owner November 5, 2024 11:54
@akashchi akashchi requested a review from mryzhov November 7, 2024 09:22
@akashchi akashchi added the WIP work in progress label Nov 11, 2024
$ovCoreWheelPath=Get-ChildItem -Path . -Filter openvino_tokenizers-*.whl | % { $_.FullName }
python3 -m pip install "$ovCoreWheelPath"
working-directory: ${{ env.INSTALL_WHEELS_DIR }}
- name: Install OpenVINO Python wheels
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest a bit different approach to find and install wheels, to simplify support in future:

For example how to call this action:
# Install Python benchmark_app by installing openvino-*.whl
- name: Install OpenVINO Python wheels
uses: ./openvino/.github/actions/install_ov_wheels
with:
wheels-dir-path: ${{ env.INSTALL_WHEELS_DIR }}
install-wheels: |
- openvino[extra]
- openvino-dev
- openvino-tokenizers
- openvino-genai

And inside the action we could use the following installation aproach:

pip install openvino[extra] openvino-dev openvino-tokenizers --find-links ${{ env.INSTALL_WHEELS_DIR }} --no-index --no-cache

@ilya-lavrenov , @akladiev what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

pip will find the suitable weel by himself and do not pickup it from PyPI

Copy link
Contributor Author

@akashchi akashchi Nov 12, 2024

Choose a reason for hiding this comment

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

There is a problem with this approach: since the idea is to use --no-index so that pip does not try to install any local wheels from PyPI instead, it cannot install the dependencies because it is forbidden from trying to access PyPI like in https://github.com/openvinotoolkit/openvino/actions/runs/11794716928/job/32855012693#step:7:43. The wheels have many dependencies that are hosted on the package index hence we cannot restrict the pip from accessing the index.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but the 1st comment is still actual, I think it is better to make action more universal (just provide the wheel pattern names as argument)

Copy link
Collaborator

@akladiev akladiev Nov 13, 2024

Choose a reason for hiding this comment

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

Actually, to make sure that a local wheel is installed, internally we pass the expected package version like this pip install openvino-dev==2025.0.0.dev20241113 --find-links.... So there is no need for --no-index. Package version is saved to our manifest file before build starts (field wheel_product_version).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we should proceed with the current approach: using the openvino openvino_tokenizers ... as the input and iterating over the given wheel names for installation.
Additionally, the openvino_tokenizers wheel has a different version for some reason:
image
so we would not be able to use it for local installation. Also, it would be rather cumbersome to pass the version from the build job to other jobs in which the wheels are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CI OpenVINO public CI github_actions Pull requests that update GitHub Actions code WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants