Skip to content

feat: adding epss data #3104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 29, 2023
Merged

feat: adding epss data #3104

merged 3 commits into from
Jun 29, 2023

Conversation

Rexbeast2
Copy link
Contributor

fixes: #3103

@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2023

Codecov Report

Merging #3104 (3dd3046) into main (6d746c7) will decrease coverage by 2.81%.
The diff coverage is 60.24%.

@@            Coverage Diff             @@
##             main    #3104      +/-   ##
==========================================
- Coverage   82.46%   79.66%   -2.81%     
==========================================
  Files         714      714              
  Lines       10984    10931      -53     
  Branches     1477     1264     -213     
==========================================
- Hits         9058     8708     -350     
- Misses       1543     1827     +284     
- Partials      383      396      +13     
Flag Coverage Δ
longtests ?
win-longtests 79.66% <60.24%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/data_sources/epss_source.py 54.16% <54.16%> (ø)
test/test_source_epss.py 100.00% <100.00%> (ø)

... and 25 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@anthonyharrison anthonyharrison left a comment

Choose a reason for hiding this comment

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

@Rexbeast2

(1) the cvedb.py file isn'r required in this PR
(2) Suggest that the test file is renameed to be something like epss_scores_test.csv
(3) Suggestion that we add a test to check that cache processing is working
(4) What happens if the file can't be downloaded/found?

@Rexbeast2
Copy link
Contributor Author

@anthonyharrison I have applied your suggestion, and for right now if the file is not downloaded or found, it will give this error
Screenshot 2023-06-26 at 11 18 30 AM
I have made it stop other things from executing right now. I will add that while populating the database.
and can you also elaborate the 3 point you said

Copy link
Contributor

@rhythmrx9 rhythmrx9 left a comment

Choose a reason for hiding this comment

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

I think you should add a test to check for downloading of epss data as well, just as @anthonyharrison suggested.

time_difference = datetime.now() - last_modified

# Check if the file is older than 24 hours
if time_difference < timedelta(hours=24):
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be if time_difference > timedelta(hours=24): as epss data should be downloaded if file is older than 24 hours?

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Since @Rexbeast2 mentioned yesterday that it would be easier if we had some stuff merged so he could work on the next piece over the weekend, and this one is at least passing all tests, I'm going to go ahead and merge this now before I'm offline for the US long weekend. If it needs a bit more work to improve error handling then it can be done in a future PR.

@terriko terriko merged commit 86e4811 into intel:main Jun 29, 2023
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.

feat: adding epss data
5 participants