add thoth-wheels to solver output

[Gregory-Pereira]

Related Issues and Dependencies

Partially addresses Solver should report exact package hash that was used to install a package #5102 (1 of 3)

This introduces a breaking change

• Yes
• No

While the change is not breaking, this feature will not work until we begin storing these wheels in Thoth, and adviser is adjusted to query wheels first in construction of lock files.

This Pull Request implements

Store SHA's for artifact that is optimized / working with a given environment.

Description

Solver will grab all available SHAs from PYPI or other index, and then will create a temporary requirements file with the dependency, dependency version and hash attempted for that dependency/version combination. It will then attempt to install that package, and if it succeeds it knows that the artifact associated with that SHA works for the given environment. It will then upload these SHAs within the thoth-wheels dict.

Currently uses brute force.

Holding for other corresponding PRs in other repos.

[Gregory-Pereira]

/hold for permissions issue with cli.py and other component adjustments for this feature.

[fridex]

Thanks for digging into this. This is really low-level and hardcore part of the system.

The approach might require some more effort to have it slightly restructured. The thing is that solver is run thousands of times in the cluster and each artifact download costs us a lot of time. This change could eventually land in the code that does the actual dependency extraction to make sure the artifact is downloaded really just once and the corresponding data are extracted from it (per artifact). It might happen that two different builds can have different metadata which we should be aware of (hence this could land in a different part of the codebase). I'm open to discuss the approach on a call if you want. Thanks again for your contribution and time spent on this.

[Gregory-Pereira]

I decided to add the index from which an artifact is retreived into thoth-wheels because the hash for an artifact is specific to where it comes from. Now it should be something like:

thoth-wheels: [
    { "<index>-<package>-<package_version>-<platform>-py<python_version>": sha256}
    ...
]

That being said I came across an issue with testing this change against indexes that arent PYPI (ie: https://pulp.operate-first.cloud). The way one can query the PYPI index is by using the index base_url (https://pypi.org), and then a repository api (/simple). However with the pulp instance on operate-first, no data is returned using a similar endpoint, instead you must pass the package name itself into the index_url, see examples:

---

No Data:

$ python3 ./thoth-solver -vvv python -r 'gym-donkeycar' --index 'https://pulp.operate-first.cloud' --no-transitive -o gym-donkeycar.json
$ python3 ./thoth-solver -vvv python -r 'gym-donkeycar' --index 'https://pulp.operate-first.cloud/pypi' --no-transitive -o gym-donkeycar.json
$ python3 ./thoth-solver -vvv python -r 'gym-donkeycar' --index 'https://pulp.operate-first.cloud/simple' --no-transitive -o gym-donkeycar.json
$ python3 ./thoth-solver -vvv python -r 'gym-donkeycar' --index 'https://pulp.operate-first.cloud/pypi/simple' --no-transitive -o gym-donkeycar.json

These commands all report:

thoth.solver.python.python:284: No versions were resolved for dependency 'gym-donkeycar' in version ''

---

Gets Data:

$ python3 ./thoth-solver -vvv python -r 'gym-donkeycar' --index 'https://pulp.operate-first.cloud/pypi/gym-donkeycar/simple' --no-transitive -o gym-donkeycar.json

This is an issue because that would mean you need a different index_url using operate-first pulp for every package requirement. I am not sure if this is configurable in operate-first/apps deployment of pulp, or if this is something thoth-station/solver will have to work around, and so I am not sure if this requires an issue, and if it does, where it should live.

All that being said I am still getting an error for the command that gets data, for some reason:

File "$HOME/.local/share/virtualenvs/solver-gT0KS2YR/lib/python3.8/site-packages/thoth_solver-1.11.2-py3.8.egg/thoth/solver/python/python.py", line 245, in _fill_hashes
    raise ValueError(f"No artifact hashes were found for {package_name}=={package_version} on {source.url}")
ValueError: No artifact hashes were found for gym-donkeycar==1.1.1 on https://pulp.operate-first.cloud/pypi/gym-donkeycar/simple

It seems that it is returning no data in _fill_hashes when it calls source.get_package_hashes(source). Maybe source.py will have to be update to support private package indexes ...

[fridex]

The issue with _fill_hashes has been fixed in thoth-python thoth-station/python#457 (try updating it in your environment). The deployment of thoth-solver is responsible to correctly propagate URLs to indexes - either pulp, PyPI or other ones that are registered in the deployment. The full listing can be seen in thamos indexes listing.

It might be a good idea to slightly revisit the approach. The problem here is that these artifacts will be downloaded multiple times, which is unnecessary. This is a quite big change with a large impact in the system - there is a need to adjust also database schema and stuff like that to fully support artifact hashes. This task is not trivial and requires deep knowledge of the system - we can sync on a call if you want.

[goern]

What do we need to get this merged?

/test aicoe-ci/prow/pytest

[sesheta]

@goern: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

• /test pre-commit
• /test thoth-pytest-py38

Use /test all to run all jobs.

Details

In response to this:

What do we need to get this merged?

/test aicoe-ci/prow/pytest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[Gregory-Pereira]

@fridex has shared some things that need to be changed. I will update it as soon as I am done adding the f35-py310 solver overlay, should be updated by end of day.

[Gregory-Pereira]

Might take a little longer, got caught up with the patch s2i patch for new f35-py310 image yesterday. In terms of visibility, the issues that I need to refactor in this before it gets merged:

• Refactor all variables into common format used in this project (ie: test_var rather than testVar or testvar)
• Find a way to grab all the hashes before installation. As we are trying to make the solver component match the PEP standard, we cannot place any requirements on pip. However, in this current implementation pip is responsible for providing the stdout that is parsed to ascertain artifact hashes from different package indexes.

[Gregory-Pereira]

Taking a new approach based on the comments about removing our reliance on the stdout produced by pip that were raised above.

Instead of running pip install off the bat, it would run pip download to ensure that if a .tar.gz is selected as the installation candidate by pip, it will not immediately unpack it (which would change our artifact hash). We then store the hash of the file using pip hash, and proceed to then test the installation of the artifact. If this succeeds we will append that hash to thoth_wheels in the way this is currently being accomplished.

I do have a question about balancing performance and functionality. If it is true that we want to know (from the client side) every artifact for which one's environment would work for, this solution would actually end up downloading each of those artifacts. It is possible to pass --platform and --python-version directives to pip download which would allow us to optimize this a bit, but it may significantly increase overhead. An alternative option is we resolve this on the Thoth side. On the client side the pip download would select the best candidate and therefore it would be likely that the pip install would succeed, but if it fails, it fails. Then on the Thoth side we would add any new occurrences of artifact hashes that succeeded.

Any thoughts @fridex?

[Gregory-Pereira]

POC has been pushed. Code isn't the cleanest but take a look at the general premise, provide feedback, and I can clean it up tomorrow.

[Gregory-Pereira]

Test passing ready for review / feedback.

[Gregory-Pereira]

Changed variable names to match standard and linting extra spaces. Tests failed but did not run in the first place.
/retest

[sesheta]

@Gregory-Pereira: The lifecycle/frozen label cannot be applied to Pull Requests.

Details

In response to this:

Thoth-station is making a priority of stabilizing the system before introducing new changes, and so this PR might hang for bit.
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sesheta]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[sesheta]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle rotten

[sesheta]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign harshad16 for approval by writing /assign @harshad16 in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sesheta]

@Gregory-Pereira: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
aicoe-ci/prow/pytest	ae5cb61	link	true	/test thoth-pytest-py38

Full PR test history. Your PR dashboard. Please help us and open an issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.