Add models for CodeFix #1704
Add models for CodeFix #1704
Conversation
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
There was a problem hiding this comment.
Thanks! Here are a few comments for your consideration.
IMHO I am not sure we want a PURL for a commit, we should consider using vcs URLs rather than a plain commit id...
I am open to something else that we can design, but if we do that we need a way to get back to the commit.
We are going to have cases that are NOT on GitHub and therefore are just plain git coordinates.
|
|
||
|
|
||
| class CodeChange(models.Model): | ||
| commits = models.JSONField(blank=True, default=list) |
There was a problem hiding this comment.
Can you add docstring for the model and help text for each field?
| null=True, | ||
| blank=True, | ||
| on_delete=models.SET_NULL, | ||
| related_name="base_version_changes", |
There was a problem hiding this comment.
base_version_changes or base_version_codechange?
|
|
||
| class CodeFix(CodeChange): | ||
| vulnerabilities = models.ManyToManyField("Vulnerability", related_name="codefixes", blank=True) | ||
| applies_to_versions = models.ManyToManyField("Package", related_name="fixes", blank=True) |
There was a problem hiding this comment.
Can you elaborate on what applies_to_versions means ?
Also I wonder if it would not better to reuse the "AffectedByPackageRelatedVulnerability" relation?
|
|
||
| created_fix_count = 0 | ||
| progress = LoopProgress(total_iterations=references.count(), logger=self.log) | ||
| for reference in progress.iter(references.paginated(per_page=500)): |
There was a problem hiding this comment.
How can we avoid to reprocess references that were already processed?
| if commit_id and package_urls: | ||
| for purl in package_urls: | ||
| normalized_purl = normalize_purl(purl) | ||
| package = self.get_or_create_package(normalized_purl) |
There was a problem hiding this comment.
Do we really want to create new packages here? I would have thought we are only adding new CodeFix?
| codefix = self.create_codefix_entry( | ||
| vulnerability=vulnerability, | ||
| package=package, | ||
| commit_id=commit_id, |
There was a problem hiding this comment.
Can craft a VCS URL instance instead? If we have a commit we need to have a base repo too, but with a vcs_url, we can combine this is one.
| return codefix | ||
| except Exception as e: | ||
| self.log(f"Error creating CodeFix entry: {e}") | ||
| return None |
There was a problem hiding this comment.
| return None | |
| return |
| urls.append(f"pkg:github/{namespace}/{name}@{commit}") | ||
| return urls | ||
|
|
||
| def extract_commit_id(self, url): |
There was a problem hiding this comment.
Can you extract that in a separate top level function so we can test all different variations?
|
|
||
| self.log(f"Successfully created {created_fix_count:,d} CodeFix entries.") | ||
|
|
||
| def extract_package_urls(self, reference): |
There was a problem hiding this comment.
ah, I see you are creating a PURL... I wonder more simply if we could instead create a vcs url as in https://github.com/aboutcode-org/scancode-toolkit/blob/c40476ab1dcf91fd92f4b2eb5ee64ca4a6aa1002/src/packagedcode/utils.py#L42 ?
Reference: #1696