python plugin: output json in pip list #1393
Conversation
LP: #1683395
| return { | ||
| package['name']: package['version'] | ||
| for package in json.loads(output) | ||
| } |
There was a problem hiding this comment.
Looks lovely, much cleaner.
Please add a test case based on the bug, with a config file init that tries to set the format.
There was a problem hiding this comment.
That would be a python integration test, which is very slow.
I'm happy here just checking that the existing tests are still passing, confirming that this change is transparent.
There was a problem hiding this comment.
I can't really agree with your points to be honest, it's just no tested at the end of the day.
Perhaps we should run python integration tests daily if it's that bad?
There was a problem hiding this comment.
We are testing that --json returns a list of packages just in the format we require, which is the only thing I'm after here, because it enables me to rely on that for the next branch about asset tracking.
We are not testing anything related to the local config file. And I'm ok with that because the config affects many more things than the output of pip list. As I mentioned on the bug, that's a separate and bigger problem that should probably be solved with your suggestion about the environment variable.
The solution for the slow integration tests is to run them in parallel. Running lxc in travis will allow us to start working on that.
There was a problem hiding this comment.
That makes sense. The way you put this PR in the context of the bug report made it seem to me as if it's addressing the bug, which isn't the case. It's just improving the parsing. A follow-up is going to be needed.
LP: #1683395