Record on the requirement whether it has been successfully downloaded #7710
Conversation
| @@ -133,7 +133,8 @@ def run(self, options, args): | |||
| ) | |||
|
|
|||
| downloaded = ' '.join([ | |||
| req.name for req in requirement_set.successfully_downloaded | |||
| req.name for req in requirement_set.requirements.values() | |||
| if req.successfully_downloaded | |||
There was a problem hiding this comment.
Q: does changing this to the following work?
| if req.successfully_downloaded | |
| if not req.editable and not req.satisfied_by |
There was a problem hiding this comment.
If it does... we could just drop the attribute basically.
There was a problem hiding this comment.
I'll give it a try. But even if it does work I think I'd want to hide it behind a successfully_downloaded() method, because that expression gives no hint that it's checking if the requirement was downloaded :-(
Then at least the poor soul who looks at that code in the future stands some chance of knowing what's going on!
There was a problem hiding this comment.
Hmm, the tests work with this change. But I did grep through the tests for "Successfully downloaded", and didn't get any hits, so I'm not actually sure we test for this.
So I'd prefer to stick with the current PR, as it's behaviourally the same as the existing code. We can add a test for the behaviour, and then refactor further, in a follow-up. For now, this decouples the download command from the RequirementSet, which is the important step at the moment.
lock
bot
added
the
auto-locked
This moves the information about whether a requirement has been successfully downloaded out of the requirement set and onto the requirement itself.
The code that determines whether a requirement has been "successfully downloaded" is in the resolver, so this initial refactoring doesn't reduce the coupling between the resolver and the client code (the
pip downloadcommand) very much. However, it does move the information the client needs from theRequirementSet(which we're trying to mke into an implementation detail of the resolver) to the requirement itself (which is what logically owns the information).A subsequent PR could then put the responsibility for determining when the requirement has been successfully downloaded onto the requirement itself.