Skip to content
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

feat: Integration with NVD API 2.0 (#2542) #2562

Merged
merged 11 commits into from
Jan 24, 2023
Merged

Conversation

anthonyharrison
Copy link
Contributor

No description provided.

@anthonyharrison
Copy link
Contributor Author

Interesting failures. Wonder if there is a differences in the data produced using the different APIs becuase when I run test_langauge.py on my API2 download, I get 4 failures (Go, Ruby, Python and R).

@terriko
Copy link
Contributor

terriko commented Jan 23, 2023

I'm pretty sure all the test fails will be fixed by #2574 (looks like commonmark got dropped by rich) but given how finicky the NVD API can be I think I'll re-run the tests here before I merge.

@terriko terriko added the awaiting maintainer Need a maintainer to respond / help out label Jan 23, 2023
@terriko
Copy link
Contributor

terriko commented Jan 24, 2023

Updating branch for tests now.

@codecov-commenter
Copy link

Codecov Report

Merging #2562 (1ed3551) into main (2263e4c) will decrease coverage by 0.26%.
The diff coverage is 2.89%.

@@            Coverage Diff             @@
##             main    #2562      +/-   ##
==========================================
- Coverage   76.91%   76.65%   -0.26%     
==========================================
  Files         594      594              
  Lines        9793     9831      +38     
  Branches     1336     1345       +9     
==========================================
+ Hits         7532     7536       +4     
- Misses       1958     1992      +34     
  Partials      303      303              
Flag Coverage Δ
longtests 76.65% <2.89%> (-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/nvd_source.py 22.17% <0.00%> (-0.65%) ⬇️
cve_bin_tool/nvd_api.py 18.32% <4.25%> (-2.93%) ⬇️
cve_bin_tool/cli.py 63.98% <0.00%> (+0.96%) ⬆️

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

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.

I think we're good to merge this. We should talk soon about whether we need a 3.2.1 release for this -- I think probably yes, but I was hoping to also fix the LegacyVersion stuff and that hasn't been completed yet. I'll file an issue about the timeout so we don't forget to look it later.

connector = aiohttp.TCPConnector(limit_per_host=19)
connector = aiohttp.TCPConnector(limit_per_host=self.max_hosts)
connection_timeout = aiohttp.ClientTimeout(
total=None, # default value is 5 minutes, set to `None` for unlimited timeout
Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly suspect we're going to want a total timeout to keep it from hanging indefinitely (specifically because this seems to happen in github actions windows instances), but I'm not sure what the value should be here. Maybe we could open a bug to look at it in future?

@terriko terriko merged commit ab7db46 into intel:main Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting maintainer Need a maintainer to respond / help out
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants