Skip to content

chore: Stop bundling optional dependency pyston and move to extras #3874

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

Closed
wants to merge 1 commit into from

Conversation

nikolaik
Copy link

@nikolaik nikolaik commented Nov 14, 2022

Description

This fixes checkov install on alpine and probably others where the version markers are unable to omit musl and similarly incompatible install targets.

checkov can still bundle pyston by installing/requiring checkov[pyston]

Fixes #3649

This can be tested by cloning this pr on a failing machine and doing pip install . and pip install .[pyston].

Some reasons for why this change is a good one:

This fixes checkov install on alpine and probably others

checkov can still bundle pyston by installing/requiring `checkov[pyston]`
@nikolaik nikolaik changed the title fix: Move optional pyston dependencies to extras fix: Stop bundling optional dependency pyston and move to extras Nov 14, 2022
@nikolaik nikolaik marked this pull request as ready for review November 14, 2022 12:22
@nimrodkor nimrodkor changed the base branch from master to main November 27, 2022 07:46
@nimrodkor nimrodkor changed the title fix: Stop bundling optional dependency pyston and move to extras chroe(general): Stop bundling optional dependency pyston and move to extras Nov 27, 2022
@gruebel gruebel changed the title chroe(general): Stop bundling optional dependency pyston and move to extras chore: Stop bundling optional dependency pyston and move to extras Nov 27, 2022
@nikolaik
Copy link
Author

@nimrodkor Should I move pyston to dev dependencies in Pipfile?

@gruebel
Copy link
Contributor

gruebel commented Nov 28, 2022

hey @nikolaik thanks for the contribution, but this basically removes pyston completely. Most users will not try to run pip install checkov[pyston] and there is no way I know of to do the opposite of adding some parameter to not install a package. We will also leverage more and more Python packages with C extensions or even add Cython in the mix, which will result in checkov not being installable on Alpine at all.

Python on Alpine is usually not recommended due to the usage of musl libc compared to glibc, which results in weird errors and degraded performance. Here you can find some info around that docker-library/python#509

Long story short, we don't target Alpine as the OS to run checkov on, if it works great, if not then use our Docker image or move to a newer Debian or Fedora based OS.

@gruebel gruebel closed this Nov 28, 2022
@nikolaik
Copy link
Author

Thanks for reviewing!

Python on Alpine is usually not recommended due to the usage of musl libc compared to glibc, which results in weird errors and degraded performance. Here you can find some info around that docker-library/python#509

I see the concern with slower performance under alpine, especially with https://superuser.com/questions/1219609/why-is-the-alpine-docker-image-over-50-slower-than-the-ubuntu-image Found another discussion on the topic here: geldata/gel#4273 In my experience I have not had any issues with running python things under alpine.

Long story short, we don't target Alpine as the OS to run checkov on, if it works great, if not then use our Docker image or move to a newer Debian or Fedora based OS.

With python 3.11 pyston deps are no longer matched using the markers, so bumping to python 3.11 when using alpine is a valid workaround for now.

As long as checkov deps provide musllinux wheels then installation will just work, even though there is not official support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkov installation on Ubuntu
2 participants