Refactoring: Simplify upload SARIF flow and reuse loaded flags#2361
Refactoring: Simplify upload SARIF flow and reuse loaded flags#2361henrymercer merged 7 commits intomainfrom
Conversation
eae5c66 to
bfdafbc
Compare
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.
src/upload-lib.ts
Outdated
| 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?
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.
|
💭 The |
|
...and it passed! So, guessing the error came from the server side. |
|
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. |
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