Skip to content

Conversation

michaelehab
Copy link
Collaborator

Solves #1981

* Add PySec Live V2 Importer

* Add tests for the PySec Live V2 Importer

* Tested functionally using the Live Evaluation API in #1969

Signed-off-by: Michael Ehab Mikhail <[email protected]>
Copy link
Collaborator

@ziadhany ziadhany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelehab Great job 👍 A few functions could be refactored to improve readability.

Comment on lines +70 to +71
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move import PySecLiveImporterPipeline to the top of the test

Suggested change
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline



def test_package_with_version_not_affected(mock_zip_data):
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline

Comment on lines +129 to +130
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from vulnerabilities.pipelines.v2_importers.pysec_live_importer import PySecLiveImporterPipeline

if not file_name.startswith("PYSEC-"):
continue
with zip_file.open(file_name) as f:
import json
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move import json to the top

Comment on lines +59 to +62
try:
v = PypiVersion(version)
except Exception:
return False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catch only InvalidVersion instead of all errors.

return False
for entry in affected:
ranges = entry.get("ranges", [])
for r in ranges:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r in for r in ranges: could be renamed to something clearer like version_range/data_range for readability.

Comment on lines +64 to +65
ranges = entry.get("ranges", [])
for r in ranges:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ranges = entry.get("ranges", [])
for r in ranges:
for event in r.get("events", []):

events = r.get("events", [])
introduced = None
fixed = None
for event in events:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for event in events:
for event in r.get("events", []):

for entry in affected:
ranges = entry.get("ranges", [])
for r in ranges:
events = r.get("events", [])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
events = r.get("events", [])

if not self._is_version_affected(advisory_dict, self.purl.version):
continue

f.seek(0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need f.seek(0) here? Could we just read the file once instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants