Skip to content

Conversation

@sbs2001
Copy link
Collaborator

@sbs2001 sbs2001 commented Nov 21, 2020

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
Copy link
Collaborator Author

sbs2001 commented Nov 21, 2020

Fixes #280

Copy link
Member

@pombredanne pombredanne left a comment

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

@sbs2001
Copy link
Collaborator Author

sbs2001 commented Nov 22, 2020

@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
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
from urllib.request import urlopen

import pytoml as toml
import toml
Copy link
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

Copy link
Member

@pombredanne pombredanne left a comment

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