refactor: cvedb structure and datasources #1706
Conversation
|
Resolved merge conflict. |
Codecov Report
@@ Coverage Diff @@
## main #1706 +/- ##
==========================================
+ Coverage 80.20% 82.12% +1.91%
==========================================
Files 300 304 +4
Lines 6427 6541 +114
Branches 1050 1063 +13
==========================================
+ Hits 5155 5372 +217
+ Misses 1054 953 -101
+ Partials 218 216 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
There was a problem hiding this comment.
This is looking pretty good. I think we should probably switch this to be data_sources throughout. I think nvd_source and curl_source are clear enough on their own (that is, I don't think nvd_data_source would be particularly clearer), but the directory and other variable names should be changed.
Other than that, some of the tests are failing and I can't explain why, so I'll leave that to you to resolve.
I also flagged a few places this is using os.path -- we're in the process of refactoring to use pathlib. I don't think it's urgent that these need to be refactored before this can be merged, but we should open up some separate refactor wishlist issues for these files.
cve_bin_tool/cli.py
Outdated
| @@ -53,6 +53,7 @@ | |||
| from cve_bin_tool.output_engine import OutputEngine | |||
| from cve_bin_tool.package_list_parser import PackageListParser | |||
| from cve_bin_tool.sbom_manager import SBOMManager | |||
| from cve_bin_tool.sources import curl, nvd | |||
There was a problem hiding this comment.
I think we'd discussed changing this to be data_sources rather than sources throughout.
cve_bin_tool/sources/__init__.py
Outdated
| from abc import ABC, abstractmethod | ||
|
|
||
| # database defaults | ||
| DISK_LOCATION_DEFAULT = os.path.join(os.path.expanduser("~"), ".cache", "cve-bin-tool") |
There was a problem hiding this comment.
We've switched to pathlib elsewhere when I merged #1714 , so this either should be switched to use pathlib or we should open an issue to do the refactor separately.
cve_bin_tool/sources/curl.py
Outdated
| data = dict(zip(headers, values)) | ||
| if data: | ||
| json_data.append(data) | ||
| filepath = os.path.abspath( |
There was a problem hiding this comment.
Another place for pathlib refactoring. Probably file a separate issue for this file?
cve_bin_tool/sources/nvd.py
Outdated
| self.error_mode = error_mode | ||
|
|
||
| # set up the db if needed | ||
| self.dbpath = os.path.join(self.cachedir, DBNAME) |
There was a problem hiding this comment.
Pathlib refactoring again.
Co-authored-by: Terri Oda <terri@toybox.ca>
|
Tests are passing, class and variable names are changed accordingly. |
|
Yup, cve scan still failing, see more in #1741. But I'm going to go ahead and merge this while we resolve that one separately. |
Separated database functionality and data fetching functionality in
CVEDBto facilitate addition of more datasources.