183602 Fix an issue with updating proper job attempt numbers on re-run#4988
Conversation
|
🤖 Hi @ievdokdm, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
This Pull Request removes the caching functionality for pull requests by number from the Scheduler and relies instead on finding the pull request by SHA from Firestore in the API handlers for rerunning jobs. While the code changes align with simplifying the caching logic, they introduce a logical flaw related to how pull requests are queried.
🔍 General Feedback
- The refactoring nicely cleans up the API handlers by unifying the logic used for rerunning failed jobs.
- Removing
pullRequestNumand relying solely on the SHA to fetch PRs introduces a potential conflict since a single SHA can be associated with multiple Pull Requests. - Please consider retaining the PR number lookup to ensure the API precisely operates on the intended pull request context.
…rSha` to `PrCheckRuns.findPullRequestFor`
|
auto label is removed for flutter/cocoon/4988, Failed to merge flutter/cocoon/4988 with Pull request flutter/cocoon/4988 could not be merged: Required status check "ci.yaml validation" is expected.. |
|
auto label is removed for flutter/cocoon/4988, Failed to merge flutter/cocoon/4988 with Pull request flutter/cocoon/4988 could not be merged: Required status check "ci.yaml validation" is expected.. |
1. Refactored Pull Request Retrieval
rerun_all_failed_jobs.dartandrerun_failed_job.darthas been updated to consistently usePrCheckRuns.findPullRequestForto resolve the pull request via the commit SHA (guard.commitSha), rather than querying by PR number.reScheduleTryBuilds.2. Removed
pull_request_numField and Cachepull_request_numfield was entirely removed from thePrCheckRunsFirestore model, including its related mappings and integration test matchers (_pr_check_run.dart).findPullRequestCachedForPullRequestNumlookup method from bothPrCheckRunsand theSchedulerservice. Additionally, removed its related caching tests.3. Service/Dependencies Cleanup
_githubServicefromscheduler.dart.dart format) to ensure code uniformity in edited files.4. Test Suite Adjustments
rerun_all_failed_jobs_test.dartandrerun_failed_job_test.dartwere updated to properly seed data usingPrCheckRuns.initializeDocument(...)mirroring production behavior.reScheduleTryBuildsreceives accurate target configurations (e.g., proper attempt increments likecontainsPair(targetA, 2)).mocks.mocks.dartand testingdata_seeder.dartto remove references to the deleted fields and getters.fix: flutter/flutter#183602