[scheduler] ci.yaml check run#1229
Conversation
| request.headers.set('X-GitHub-Event', 'pull_request'); | ||
| }); | ||
|
|
||
| test('Exception is raised when no builders available', () async { |
There was a problem hiding this comment.
This is no longer true as there's always at least one check we generate on presubmit
| commitSha: commitSha, | ||
| ); | ||
| } catch (e) { | ||
| exception = e; |
There was a problem hiding this comment.
Should we log this exception?
There was a problem hiding this comment.
we should also only catch a more specific type of Exception, rather than everything.
There was a problem hiding this comment.
There was a problem hiding this comment.
I added a log.
We could catch FormatException and HttpException for ci.yaml related issues, but I figured if there's an issue in scheduling the LUCI builds (anywhere), we should mark it as failed. That way people will retry the presubmit instead of having a forever pending checkrun. WDYT?
There was a problem hiding this comment.
Moved to catch on FormatException. If another exception fails, this will leave the ci.yaml validation as pending.
There was a problem hiding this comment.
Will the exception bubbling up lead to a stronger signal that monitoring will detect? If not, then I suppose there's no advantage to disambiguating FormatExceptions.
| ); | ||
| } else { | ||
| // Failure when validating ci.yaml | ||
| await githubChecksService.githubChecksUtil.updateCheckRun( |
There was a problem hiding this comment.
we can move this part to the catch section and remove the if - then - else
There was a problem hiding this comment.
catch blocks cannot be async which is why this is separated
| // Update validate ci.yaml check | ||
| if (exception == null) { | ||
| // Success in validating ci.yaml | ||
| await githubChecksService.githubChecksUtil.updateCheckRun( |
There was a problem hiding this comment.
We can add this to the end of the try block and remove the if - then else.
There was a problem hiding this comment.
This is separate to minimize the code the try block deals with (only scheduler config validation + schedule builds), otherwise we would silently fail on quota issues
CaseyHillers
added
the
waiting for tree to go green
4 participants
This adds a check to all Cocoon scheduled presubmits that validates the ci.yaml + validates all builds were scheduled.
If
.ci.yamldoes not exist, it will pass as it uses an emptySchedulerConfig.Issues
flutter/flutter#82303