feat: adding EPSS data #3130
feat: adding EPSS data #3130
Conversation
|
I'm going to rebase this because I think #3122 might be related to the errors you're getting. |
Codecov Report
@@ Coverage Diff @@
## main #3130 +/- ##
==========================================
+ Coverage 82.34% 82.56% +0.22%
==========================================
Files 716 716
Lines 11051 11090 +39
Branches 1482 1488 +6
==========================================
+ Hits 9100 9157 +57
+ Misses 1570 1553 -17
+ Partials 381 380 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
I think now is a good time to fix insert_queries() by getting rid of it entirely. More detail in an inline comment.
(Feel free to reach out to another mentor for help understanding what I'm suggesting if you need to, especially since I'm out on Fridays and you'll probably see this after I'm offline for the weekend. My feelings won't be hurt!)
| ) | ||
| VALUES (?, ?) | ||
| """ | ||
| return ( |
There was a problem hiding this comment.
Okay, so I know the insert_queries() function is more than a little my fault, but I think now is a good time to refactor. It looks weird enough with 3 outputs but with 5 it's starting to get hard to follow.
I think the right solution is to replace this function with a class variable, since it's not going to change per instance of CVEDB.
So you'd have something like this up at the top of the class (only with the other three tables included and not just your new ones):
INSERT_QUERIES = {
"cve_metrics": """
INSERT or REPLACE INTO cve_metrics (
cve_number,
metric_id,
metric_score,
metric_field
)
VALUES (?, ?, ?, ?)
""",
"metrics": """
INSERT or REPLACE INTO metrics (
metrics_id,
metrics_name
)
VALUES (?, ?)
""",
}Then you'd delete the insert_queries() function entirely. So when you wanted to get an insert from somwhere instead of doing something like...
(insert_severity, _, _, _, _) = self.insert_queries()You'd have something more like...
insert_severity = CVEDB.INSERT_QUERIES["severity"]or I guess if you think it's more readable it could be...
insert_severity = cls.INSERT_QUERIES["severity"]or honestly you could probably use the class variable directly rather than making a local copy, so you'd just go around replacing insert_severity with CVEDB.INSERT_QUERIES["severity"]
We could also do something fancy in the few functions where we actually iterate over all the tables, but I feel like that's overkill for this PR and since we're only talking 5 tables it's probably unnecessary refactoring right now.
There was a problem hiding this comment.
@terriko ok sure, I will update the INSERT_QUERIES.
Please let me know if you're sure about adding it to this PR. I could include this in the new PR and send it by tomorrow as this is a small change, and I want this PR to focus on EPSS data mainly. Either way, I am open to working on any.
There was a problem hiding this comment.
I was hoping you'd get a chance to work on the refactor while I was offline, but since that didn't happen I'll merge this and open a refactoring issue.
| ) | ||
| VALUES (?, ?) | ||
| """ | ||
| return ( |
There was a problem hiding this comment.
I was hoping you'd get a chance to work on the refactor while I was offline, but since that didn't happen I'll merge this and open a refactoring issue.
closes: #3076