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.

Sign up for GitHub

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

Adding Python venv #78

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Adding Python venv #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/kuksa_databroker-cli_build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ jobs:
~/.cache/pip/
target-*/
key: databroker-cli-release-${{ matrix.platform.name }}-${{ hashFiles('**/Cargo.lock') }}
- uses: actions/setup-python@v5
# Needed for pip
with:
python-version: '3.12'
Comment on lines +99 to +102
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really using a pip virtual environment? It looks like the problem is fixed, by installing a separate python installation which prevents the system python installation from being polluted - which might be a workable solution.

That the fix works is obvious. Just want to have a common understanding here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guess you are right - as I understand it installs Python and adds it to path. It does not in itself initiate/activate a virtual environment. Setup-python has however some support for caching so in one way I think it may reuse certain parts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some other repos like https://github.com/eclipse-kuksa/kuksa-python-sdk/blob/main/.github/workflows/dash.yaml we actually use a virtual environment. From practical perspectives I generally do no think that the difference will be that big as we typically do not need multiple different Python environments in the same workflow.

Copy link
Contributor

@wba2hi wba2hi Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Guess you are right - as I understand it installs Python and adds it to path. It does not in itself initiate/activate a virtual environment. Setup-python has however some support for caching so in one way I think it may reuse certain parts

Agree. But caching seems to be disabled in our case, because from what I understand it needs to be explicitely enabled by using:

cache: 'pip' // pipenv //

see: https://github.com/actions/setup-python?tab=readme-ov-file#caching-packages-dependencies

Again, not telling here that we have to use a cache or not use cache (both will solve problems and create others), just pointing out things :)

In some other repos like https://github.com/eclipse-kuksa/kuksa-python-sdk/blob/main/.github/workflows/dash.yaml we actually use a virtual environment. From practical perspectives I generally do no think that the difference will be that big as we typically do not need multiple different Python environments in the same workflow.

I implemented that stuff based on some commands which were provided to me, so take it with a grain of salt :D It's probably still not the best way to handle that stuff. However in that case it was mandatory to have a clean / unpolluted python instance because otherwise we would point out libraries which are required by the system as libraries used by our software.

Personally I'm fine with the approach used here. As said, I just wanted to have a common understanding. The Commit Title somehow suggests that we use a venv and maybe even what we have now is called a venv - I'm no Python expert here. I was just directly thinking about the pip venv thingy and was somehow missing in my head and in this commit where it's activated :)

- name: Install build prerequisites
working-directory: ${{github.workspace}}/
run: |
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/kuksa_databroker_build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ jobs:
~/.cargo/.crates2.json
target-*/
key: databroker-release-${{ matrix.platform.name }}-${{ hashFiles('**/Cargo.lock') }}
- uses: actions/setup-python@v5
# Needed for pip
with:
python-version: '3.12'
- name: Install build prerequisites
working-directory: ${{github.workspace}}/
run: |
Expand Down