Skip to content
New issue

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

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

Already on GitHub? # to your account

micropipenv should handle issues caused by relative ordering of dependencies #12

Closed
fridex opened this issue Feb 25, 2020 · 6 comments · Fixed by #13
Closed

micropipenv should handle issues caused by relative ordering of dependencies #12

fridex opened this issue Feb 25, 2020 · 6 comments · Fixed by #13
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. human_intervention_required

Comments

@fridex
Copy link
Collaborator

fridex commented Feb 25, 2020

Describe the bug

As reported in thoth-station/s2i-thoth#39 by @4n4nd

micropipenv does not handle the relative ordering of dependencies.

To Reproduce
Steps to reproduce the behavior:

  1. Reach out to Build fails in new v0.9.0 s2i-thoth-ubi8-py36 base image s2i-thoth#39
  2. Check build log and installed dependencies
  3. See error

Expected behavior

micropipenv should correctly install the dependencies

Additional context

As the stack supplied is already fully resolved the fix might be pretty simple - instead of issuing one pip installation, we can issue pip install for each package in the stack iteratively with --no-deps flag set (and fallback respecting errors).

@fridex fridex added bug good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Feb 25, 2020
@fridex
Copy link
Collaborator Author

fridex commented Feb 25, 2020

As the stack supplied is already fully resolved the fix might be pretty simple - instead of issuing one pip installation, we can issue pip install for each package in the stack iteratively with --no-deps flag set (and fallback respecting errors).

As we might encounter similar issues when pip-tools style requirements are used, we should be capable of handling these errors in Python s2i even for requirements.txt/requirements.in.

@4n4nd
Copy link

4n4nd commented Feb 25, 2020

What if you add one more retry for the failed dependencies, but after all other builds have finished?

@fridex
Copy link
Collaborator Author

fridex commented Feb 25, 2020

What if you add one more retry for the failed dependencies, but after all other builds have finished?

+1, that's what I tried to express

Currently, we install all the dependencies by exec to pip - we can install them one after another and retry the failed ones.

@4n4nd
Copy link

4n4nd commented Feb 25, 2020

Ah okay. But why the —no-deps flag?

@fridex
Copy link
Collaborator Author

fridex commented Feb 26, 2020

Ah okay. But why the —no-deps flag?

See #10

And more importantly - the stack present in Pipfile.lock is already fully resolved with all the dependencies pinned to specific versions. If we would install packages without --no-deps flag iteratively, they will also include their dependencies (latest possible) and that's what we don't want - we want to install exactly those packages that are present in the Pipfile.lock.

Another solution is to use --no-deps and subsequently force reinstall on dependencies to have them in other versions - 2 main drawbacks:

  • we can download a package multiple times (once as a dependency, once a a "force reinstall")
  • we can possibly install a package that should not be present - the package could be introduced by a package that we force reinstalled

Let me know if that's clear, I can come up with some example to make it more clear.

@4n4nd
Copy link

4n4nd commented Feb 26, 2020

Oh yes that makes a lot of sense.
Thanks!

@fridex fridex self-assigned this Feb 26, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. human_intervention_required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants