Skip to content

Adapt rust importer to new advisory format - #281

Merged
sbs2001 merged 10 commits into
aboutcode-org:mainfrom
sbs2001:rust_new_format
Dec 15, 2020
Merged

Adapt rust importer to new advisory format#281
sbs2001 merged 10 commits into
aboutcode-org:mainfrom
sbs2001:rust_new_format

Conversation

@sbs2001

@sbs2001 sbs2001 commented Nov 21, 2020

Copy link
Copy Markdown
Collaborator

Rust advisory db has moved on from using plain toml to using markdown + toml 'front matter' .
This PR enables the current rust importer to import data from the new format.

@sbs2001

sbs2001 commented Nov 21, 2020

Copy link
Copy Markdown
Collaborator Author

Fixes #280

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!
See some comments inline. Also try to avoid the big test files. Testting the opening of a zip file is rather trivial. One the other hand having a few test markdown as text files would make sense.

+515 KB vulnerabilities/tests/test_data/advisory-db.zip
+365 KB (170%) vulnerabilities/tests/test_data/rust-advisory-db.zip

Comment thread vulnerabilities/importers/rust.py Outdated
Comment thread vulnerabilities/importers/rust.py Outdated
Comment thread vulnerabilities/importers/rust.py Outdated
@sbs2001

sbs2001 commented Nov 22, 2020

Copy link
Copy Markdown
Collaborator Author

@pombredanne about zip files :

Those are compressed git repositories. I tried trimming these but I was getting mysterious bugs and just gave up on that. We have rust-advisory-db.zip and advisory-db.zip instead of a single rust-advisory-db.zip because the tests in test_data_source.py require the old kind of rust-advisory-db.

@pombredanne

Copy link
Copy Markdown
Member

re:

Those are compressed git repositories. I tried trimming these but I was getting mysterious bugs and just gave up on that.

IMHO we should be able to create smaller zip with fewer files. How did you do this?

We have rust-advisory-db.zip and advisory-db.zip instead of a single rust-advisory-db.zip because the tests in test_data_source.py require the old kind of rust-advisory-db.

Then may be renaming these to have a clearer name would help.

@sbs2001
sbs2001 requested a review from pombredanne November 23, 2020 15:27
Comment thread vulnerabilities/importers/rust.py Outdated
from urllib.request import urlopen

import pytoml as toml
import toml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: sort the imports

Comment thread vulnerabilities/importers/rust.py

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I made a small suggestion for your consideration. LGTM otherwise!
Thank you

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
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.

3 participants