-
Notifications
You must be signed in to change notification settings - Fork 325
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
hotfix: Pipenv version lock #383
Conversation
Can one of the admins verify this patch? |
5 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
This seems to bring back a discussion we have had earlier: we should not install parts of the toolchain during build of the application. Could we think about moving the required tools into the container's build process/time rather than application build? |
Since I wasn't part of the discussion earlier I may not see the whole context here. However, my 2 cents: if we want and expect to deliver reproducible and deterministic builds we shouldn't count on dynamic installation of the tools marketed as provided by the builder image. If I rerun an older build - based on the exact same s2i Python revision as before, same sha digest, the same app source commit hash, the same dependencies locked - the assemble script will still may go and install a different pipenv version. That makes the app build process nondeterministic. In my eyes it should handle the provided tools in the same way as the builder image "locks" and bakes in the Python micro version releases. The other thing is, there was no new pipenv version for 2 years. I totally can see why it was counted on as a stable, solid thing... |
We had this discussion almost exactly 2 years back in #278 and #290, both still open. +1 on my side - the builder container image should come with all the tools necessary to build the application without relying on PyPI. An example could be builds happening in an isolated environment without any access to Internat and installing packages solely from the intranet (see also linked issues stating broken releases of Pipenv itself that blocked our development for quite some time at that point).
+1 on both paragraphs
The situation around Pipenv was different. Anyway, what are the arguments not to provide the ability to install dependencies using Pipenv files by default in Python s2i container images? As Pipenv is in PyPA, it looks alive again now and Pipenv files seem to be the way PyPA is expecting lock files should look like? I can imagine size could be an issue, but we have a solution for that (that can help also when installing requirements from requirements.txt as micropipenv can resolve issues caused by relative ordering of dependencies installed). EDIT: Also linking #368 |
This is a real problem we have to solve. Unfortunately, neither pipenv nor micropipenv is available in RHEL/Centos so we cannot install them from RPMs in all our container images. Moreover, pipenv has a lot of dependencies and even more bundled dependencies so maintain a package like that in RHEL is painful and nobody would want to include it there. Fixing CVEs in bundled libraries is a thing I don't want to spend my time on more than necessary. Unfortunately, we cannot install these tools from PyPI during the build of the container images because our build platforms have no internet access during the build. But yes, we have to find a way how to avoid problems like this one. The best of what we can do now is to fix pipenv upstream which would also fix all of our containers. I'm gonna report the issue upstream. Then we can lock pipenv to specific version but we also have to find a way how to test the images with the latest versions. We'll try to find a better solution. |
I understand that maintaining different solutions for RHEL/Centos and Fedora adds additional overhead, but long-term accomodating micropipenv once it eventually goes to RHEL IMHO could be an option to consider.
This was one of the main reasons why micropipenv was introduced.
To my understanding that's how even the current build process works. Pipenv won't be installed if there is no internet access as it is installed during application assembling (not during the actual build of the builder container image). Hence, users without Internet access won't have any support for projects that use Pipenv files for managing dependencies.
Even if they do fix this issue, the issue described above and in #278 will remain. |
That definitely is. I'd like to see micropipenv in RHEL and no installations from PyPI in assemble script.
If users of s2i don't have internet access, they usualy provides their own packages index and this index might include also pipenv and its dependencies. This approach is not flawless now but we have a plan how to fix it.
The issue is already fixed upstream but we'll still lock the version and try to find a better solution. |
[test] |
[test-openshift] |
Is there anything needed from our side to support this idea?
What is the current plan? |
We'll have to discuss it and find answers to some questions like:
The assemble script will try to install [micro]pipenv without |
Makes sense, please keep us in any related discussion. 👍 |
[test] |
CI looks green also for RHEL images so I'm gonna merge this PR and update Fedora images. Thank you! |
Bodhi updates:
It would be awesome if somebody can test them and give them a karma points so they can reach stable registry soon. |
Fixes #382 by locking pipenv version to 2018.11.26 which is a previous release to the current latest
If there's a better, more robust fix, please close this PR in favor of that one.
This change is abusing the
install_tool
's$VENV_DIR/bin/pip --isolated install -U $1$2
notation.