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

Add Python packages to docker base image #14847

Closed
1 of 2 tasks
r0qs opened this issue Feb 12, 2024 · 8 comments
Closed
1 of 2 tasks

Add Python packages to docker base image #14847

r0qs opened this issue Feb 12, 2024 · 8 comments
Labels
build system 🏗️ closed due inactivity The issue/PR was automatically closed due to inactivity. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. should have We like the idea but it’s not important enough to be a part of the roadmap. stale The issue/PR was marked as stale because it has been open for too long.

Comments

@r0qs
Copy link
Member

r0qs commented Feb 12, 2024

@r0qs r0qs changed the title Update Python packages within the docker base image Add Python packages to docker base image Feb 12, 2024
@r0qs r0qs added low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. should have We like the idea but it’s not important enough to be a part of the roadmap. labels Feb 12, 2024
@cameel
Copy link
Member

cameel commented Feb 12, 2024

Maybe we should also include the #14839 (comment) package.

Yeah, if it's not already there. Our Python tests require it and they work so I assume it's there.

Choose the default Python version in the base image

An important part of that is to then switch from python3 to python where needed and remove the hard-coded Python version (3.12) from the Windows Python job.

@r0qs
Copy link
Member Author

r0qs commented Feb 12, 2024

Yeah, if it's not already there. Our Python tests require it and they work so I assume it's there.

They are not, most of the python packages are installed per-job/step basis. For example: https://github.com/ethereum/solidity/blob/develop/.circleci/config.yml#L914 and https://github.com/ethereum/solidity/blob/develop/.circleci/config.yml#L1441

@cameel
Copy link
Member

cameel commented Feb 12, 2024

Interesting. t_ubu_pyscripts does not install it even though unit tests require it (at least they did when I ran them locally in a clean virtualenv). It's actually weird because it randomly failed in my PR a few times due to missing requests but it went away after some reruns.

By the way, we should add those packages listed for chk_pylint to the base image too. The less we have to install at runtime in CI, the better, unless something has to be really bleeding edge like Foundry.

@nikola-matic
Copy link
Collaborator

Add parsec and tabulate python packages to base image, as noted here. Maybe we should also include the #14839 (comment) package.

Should we not just have a requirements.txt file and install the same dependencies everywhere. At the moment, requests is not present in the Windows images, but is in all of the other ones.

@cameel
Copy link
Member

cameel commented Feb 13, 2024

Maybe. The issue is that we require quite a few packages, some of which could be quite heavy or exotic but each script individually needs only one or two of them.

Another complication is that I can easily see us adding something to the requirements.txt but forgetting to update the images so maybe the image would be better place to list them.

But overall I'm not against the idea if we can solve these problems. Having a common base that we can rely on being always present would also be pretty convenient. I'm usually reluctant to add dependencies when writing one-off Python scripts because ensuring that I have them everywhere I need it is cumbersome.

@nikola-matic
Copy link
Collaborator

Maybe. The issue is that we require quite a few packages, some of which could be quite heavy or exotic but each script individually needs only one or two of them.

Another complication is that I can easily see us adding something to the requirements.txt but forgetting to update the images so maybe the image would be better place to list them.

But overall I'm not against the idea if we can solve these problems. Having a common base that we can rely on being always present would also be pretty convenient. I'm usually reluctant to add dependencies when writing one-off Python scripts because ensuring that I have them everywhere I need it is cumbersome.

By the way, your PR that was failing previously (3.11) is now passing (3.12). I've opened a draft PR #14873 to force install python 3.12 via chocolatey, and provide a symlink from that. Not exactly the prettiest approach, but should provide a stable fix for the time being.

Copy link

github-actions bot commented Aug 8, 2024

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Aug 8, 2024
Copy link

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Aug 15, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system 🏗️ closed due inactivity The issue/PR was automatically closed due to inactivity. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. should have We like the idea but it’s not important enough to be a part of the roadmap. stale The issue/PR was marked as stale because it has been open for too long.
Projects
Archived in project
Development

No branches or pull requests

3 participants