Add dependency parser task for maven #536
Conversation
abs51295
force-pushed
the
dependency-parser
branch
3 times, most recently
from
bca96f0 to
6d0d7a1
Compare
|
@abs51295 Your image is available in the registry: |
1 similar comment
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
6d0d7a1 to
471093c
Compare
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
471093c to
fbd2179
Compare
f8a_worker/process.py
Outdated
| @@ -42,13 +42,14 @@ def config(): | |||
| "/usr/bin/true"]) | |||
|
|
|||
| @classmethod | |||
| def clone(cls, url, path, depth=None, branch=None, single_branch=False): | |||
| def clone(cls, url, path, timeout, depth=None, branch=None, single_branch=False): | |||
There was a problem hiding this comment.
timeout should be optional
There was a problem hiding this comment.
Changing it to keyword parameter with default value of 300 since that's the default for get_command_output as well.
| github_repo = arguments.get('github_repo') | ||
| github_sha = arguments.get('github_sha') | ||
| self._strict_assert(arguments.get('github_repo')) | ||
| self._strict_assert(arguments.get('github_sha')) |
There was a problem hiding this comment.
You want to do the asserts first, i.e.
self._strict_assert(arguments.get('github_repo'))
self._strict_assert(arguments.get('github_sha'))
github_repo = arguments['github_repo']
github_sha = arguments['github_sha']| import re | ||
|
|
||
|
|
||
| class DependencyParserTask(BaseTask): |
There was a problem hiding this comment.
Maybe GithubDependencyTreeTask ?
There was a problem hiding this comment.
Not really sure about that. I think current name is good enough.
There was a problem hiding this comment.
Well, we already have a DependencyParser, which is why I suggested the change, but I'll leave that up to you.
There was a problem hiding this comment.
Oh thanks for pointing that out. Would change it accordingly.
| return parse_maven_dependency_tree(f.readlines()) | ||
|
|
||
|
|
||
| def parse_maven_dependency_tree(dependency_tree): |
There was a problem hiding this comment.
Can this and extract_dependencies() be (static) methods of the task class ?
| "-DappendOutput=true"] | ||
| timed_cmd = TimedCommand(cmd) | ||
| timed_cmd.run(timeout=3600) | ||
| with open("dependency-tree.txt") as f: |
There was a problem hiding this comment.
if timed_cmd.status != 0 or not os.path.isfile("dependency-tree.txt"):
err="mvn dependency:tree failed"
self.log.error(err)
raise FatalTaskError(err)There was a problem hiding this comment.
Just learned today that it's better to use pathlib instead of os.path. Would be updating accordingly.
| group_id=artifact_coords.groupId, | ||
| artifact_id=artifact_coords.artifactId, | ||
| version=artifact_coords.version | ||
| )) |
There was a problem hiding this comment.
Can be extracted into a function:
def add_name_to_set(matching_line, names_set):
artifact_coords = MavenCoordinates.from_str(matching_line)
names_set.add("{group_id}:{artifact_id}:{version}".format(
group_id=artifact_coords.groupId,
artifact_id=artifact_coords.artifactId,
version=artifact_coords.version))
if len(matching_lines_list) == 1:
add_name_to_set(matching_lines_list[0], set_pom_names)
else:
for matching_line in matching_lines_list:
add_name_to_set(matching_line, set_package_names)There was a problem hiding this comment.
Makes sense. Would refactor it :)
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
fbd2179 to
4460bcb
Compare
|
[test] |
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
4460bcb to
0cafd96
Compare
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
0cafd96 to
d42bd95
Compare
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
d42bd95 to
eb52e3d
Compare
|
@abs51295 Your image is available in the registry: |
abs51295
force-pushed
the
dependency-parser
branch
from
eb52e3d to
f051e9f
Compare
|
[test] |
2 similar comments
|
[test] |
|
[test] |
|
@abs51295 Your image is available in the registry: |
abs51295
changed the title
abs51295
force-pushed
the
dependency-parser
branch
from
f051e9f to
5fe8bf0
Compare
GithubDependencyTreeTask performs dependency parsing currently for maven ecosystem by using mvn dependency:tree plugin.
abs51295
force-pushed
the
dependency-parser
branch
from
5fe8bf0 to
1a4c742
Compare
|
[test] |
|
Would be nice to have some unit test(s), but LGTM in general. Do you want me to merge ? |
|
@jpopelka we are planning to add that. Would send a separate PR for that. Please merge this for now. Thanks :) |
Resolves task 3 of #2326.