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: retry if NVD API Key is invalid #1574

Merged
merged 4 commits into from Mar 14, 2022
Merged

feat: retry if NVD API Key is invalid #1574

merged 4 commits into from Mar 14, 2022

Conversation

terriko
Copy link
Contributor

@terriko terriko commented Feb 9, 2022

This is a draft that shouldn't be working yet, but comments, review and suggestions are very welcome because I'm currently blocked on what to do next.

I'm still trying to figure out the best way to handle unsetting the NVD API Key if it proves to be invalid.

This version just has it retry, but I'm not convinced the parameter setting works as expected (I think we need to change it in the request) and the second attempt is slow.

@imsahil007
Copy link
Contributor

terriko#3

@terriko
Copy link
Contributor Author

terriko commented Feb 22, 2022

Merged @imsahil007 's suggested PR. (thought I did this before I was out for a US long weekend, but apparently not!) I think that's the right direction to go. We may still have issues with the actual CI.

@terriko
Copy link
Contributor Author

terriko commented Mar 9, 2022

There was a syntax error in the apiKey check, and I've updated the pull request to the latest tree. the nvd tests pass for me locally, so that's a good sign?

@terriko terriko force-pushed the nvd_ci branch 2 times, most recently from 64c0446 to 37a1847 Compare March 9, 2022 20:52
@terriko
Copy link
Contributor Author

terriko commented Mar 9, 2022

re-wrote commit messages so gitlint would quit complaining.

imsahil007 and others added 2 commits March 9, 2022 12:57
fix: add condition to check whether apiKey is being used
@terriko terriko marked this pull request as ready for review March 10, 2022 18:12
@terriko
Copy link
Contributor Author

terriko commented Mar 10, 2022

@imsahil007 @BreadGenie @anthonyharrison
This is ready for review. The good news is that the code seems to work. The bad news is that the CI pass-through of the key does not, so it's not actually using the key, and the nvd long test that's occasionally a problem is still failing.

From my reading of the docs, this may be expected behaviour and the key secret will pass only when this runs on main. Once this passes code review, I'd like to merge it and we'll work on improved key passthrough in a separate pull request.

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.

Looks OK to me

@terriko
Copy link
Contributor Author

terriko commented Mar 14, 2022

Okay, I'm going to go ahead and merge this since @anthonyharrison 's had a chance to review it and found no concerns. Further refinement after merging is always welcome!

@terriko terriko merged commit f7b4930 into intel:main Mar 14, 2022
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.

None yet

3 participants