fix: Parse multiple requirements from a poetry dependency #4179
Conversation
| # @param req can be an Array, Hash or String that represents the constraints for a dependency | ||
| def parse_requirements_from(req, type) | ||
| [req].flatten.compact.map do |requirement| | ||
| next if requirement.is_a?(Hash) && (UNSUPPORTED_DEPENDENCY_TYPES & requirement.keys).any? |
There was a problem hiding this comment.
Just double-checking, this is functionally the same as next if req.is_a?(Hash) && UNSUPPORTED_DEPENDENCY_TYPES.any? { |t| req.key?(t) } right?
There was a problem hiding this comment.
Yep. It's a set intersect operation.
irb(main):001:0> [1, 2, 3] & [2, 3]
=> [2, 3]
irb(main):002:0> [1, 3, 4] & [5, 6]
=> []
| check_requirements(requirement) | ||
|
|
||
| { | ||
| requirement: requirement.is_a?(String) ? requirement : requirement["version"], |
There was a problem hiding this comment.
Slightly paranoid that we might get something that's not a String but also does not respond to #[] in a way that we'd expect but since no tests break that might not be a concern. Should we handle the type of req more explicitly?
Totally fine to keep as is, just thinking out loud here, but I found the [req].flatten.compact.map do |requirement| a little hard to follow 🤔
There was a problem hiding this comment.
Slightly paranoid that we might get something that's not a String but also does not respond to #[] in a way that we'd expect but since no tests break that might not be a concern.
I think this is a valid concern. Matching on type signature feels icky to me. I feel like there's a missing abstraction/interface here but I'm not sure what that is.
Should we handle the type of req more explicitly?
I'm not against this idea. I worked on this issue during my on-call shift last week so I've lost some of the context due to mo's limited memory but I can circle back to this. I think I would prefer to ship this small change as is because there were some jobs failing with this issue so this would at least unblock those. WDYT?
Why is this needed?
Poetry projects can specify multiple requirements for a dependency.
e.g.
This change is needed to allow the parser to parse dependencies with multiple
requirements.
What does this do?
This change handles poetry dependencies with multiple requirements by looping
through each requirement specified for a dependency.
Type of change