Refactoring: Simplify upload SARIF flow and reuse loaded flags - #2361
henrymercer merged 7 commits into
Conversation
Sorry, something went wrong.
aeisenberg
left a comment
There was a problem hiding this comment.
Looks like a general improvement, but just want to make sure about the payload change.
Sorry, something went wrong.
| analysisKey, | ||
| analysisName, | ||
| category, |
There was a problem hiding this comment.
Previously, this was analysisName, but that doesn't look right since the util.getRequiredEnvParam("GITHUB_WORKFLOW") was being passed in. Now you're using category. This seems to be correct now, but will that mess up anything with our teleemtry since we're changing what we're stuffing into the analysis_name field of the payload?
Sorry, something went wrong.
There was a problem hiding this comment.
You're right, let's keep it the same. My understanding is the category in the SARIF overrides what we have in the analysis_name field, and GITHUB_WORKFLOW gives the job name, which we could use as a fallback. We might be able to stop specifying the analysis_name field if we're sure we're always specifying a category in the SARIF, but I'll leave that cleanup for later.
Sorry, something went wrong.
|
💭 The submit SARIF after failure workflow on main seems to be failing now after this was merged 😢 (though I'm not sure why it didn't fail in this PR's checks). |
Sorry, something went wrong.
|
...and it passed! So, guessing the error came from the server side. |
Sorry, something went wrong.
|
This was due to an internal experiment on the API endpoints (unrelated to this PR). The relevant feature flag is now disabled. I've cced you in Slack. |
Sorry, something went wrong.
Minor refactoring to simplify the flow for uploading SARIF files, and to reuse the loaded feature flags if we've already loaded them from disk or from the API.
Merge / deployment checklist