Fix/scorecard silent failure - #453
Draft
EngCaioFonseca wants to merge 2 commits into
Draft
Conversation
… data Signed-off-by: Caio Fonseca <engcaiofonseca@protonmail.com>
Signed-off-by: Caio Fonseca <engcaiofonseca@protonmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| p = subprocess.run(subprocess_arr,capture_output=True, text=True, timeout=None) | ||
|
|
||
| try: | ||
| p = subprocess.run(subprocess_arr,cwd=cwd,capture_output=True, text=True, timeout=timeout) |
Contributor
|
Can you fill in the AI disclosure template with the tools you used and how? |
Contributor
|
also i dont think this is a complete fix to #222, but its a good start - being able to see the errors will help us diagnose why there may not be new data showing up for scorecard |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The issue seemed to have started once in one previous commit (f6ad816), where a failed scorecard has been made silent (no logs were being written on why the scorecard is empty). It only checks if there's returned output check, if there is, even if it is empty, it considers a success. So it goes to the next task.
The fixes were re-enabling the logs, get the proper error handling, setting a time limit for the report, and actually writing the task as a fail instead of a success if it didn't work, instead of "done".
We should observe way more failures now, which is expected, since a lot of failures were happening and were just written down as successes. Which will make it possible for us to see why they are failing.
The commit issue (f6ad816):
it changed required_output['checks'] -> required_output.get('checks'). Before that, an empty result raised KeyError: 'checks' and the task would fail, that's the MetadataException (" 'checks' | Additional metadata: required_output: {}").
The .get() converted a failure into a silent success.
It is not possible to see the cause in the logs because p.stderr is thrown away. Scorecard writes all of its error details to stderr, so production logs contain only 'No scorecard checks found!' with no reason.
This PR fixes #
#222
Notes for Reviewers
Signed commits
Generative AI disclosure
Please select one option:
If AI tools were used, please provide details below:
- What tools were used?
- How were these tools used?
- Did you review these outputs before submitting this PR?