Refactor: move get_locked() from VersionSolver to Provider #6204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
radoering
force-pushed
the
refactor-solver-and-provider
branch
2 times, most recently
from
52e2ca4
to
9f8ed55
Compare
…sed in tests and can cause inconsistencies between a solver and its provider)
…lways be set via contextmanager "use_environment" to avoid inconsistent python_constraint)
…ionSolver to Provider
radoering
force-pushed
the
refactor-solver-and-provider
branch
from
9f8ed55
to
0f50a9e
Compare
neersighted
approved these changes
Aug 31, 2022
2 tasks
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactoring is motivated by #6131. While the change in #6131 seems to be functionally correct, I don't like it from an architectural point of view (passing a
get_lockedfromVersionSolvertoProvider). However, for the fixProviderhas to know about locked packages. After careful consideration, I came to the conclusion that it makes more sense, ifProviderprovides theget_lockedmethod sinceProvideralready has different methods that take a dependency as input and give one ore more packages. Further, it already has knowledge about installed packages so it makes sense to extend that to locked packages.To put it in a nutshell, the main objective of this PR is to move
get_locked()fromVersionSolvertoProvider. On the way, I noticed some other shortcomings I changed in separate commits.In case anyone is wondering why the
installedandlockedfixtures intest_solvercan't be used anymore: SinceProvidertransforms the list of locked packages into a dict, changes to the list of locked packages after creating aProviderinstance have no effect. Since changing locked packages after creating the solver/provider is no real use case (was only done in tests), I decided to adapt the tests in favor of a more clear interface that fits better to the real world use case.