Function rename deletion sequence change#10317
Function rename deletion sequence change#10317shettyvarun268 wants to merge 3 commits intofirebase:mainfrom
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Fabricator to implement a two-phase deployment process, ensuring all function creations and updates across regions are completed before any deletions are initiated. Deletions are now conditionally skipped if failures occur during the first phase. Feedback focuses on strengthening the failure detection logic to account for rejected promises, refactoring the applyChangeset signature to make scraper parameters optional during the delete phase for better clarity, and improving error reporting to ensure the deployment summary remains complete even when regional operations fail to start.
There was a problem hiding this comment.
Code Review
This pull request refactors the Fabricator to implement a two-phase deployment strategy for Cloud Functions, ensuring all creations and updates across regions complete before deletions begin. Deletions are now skipped if any preceding operations fail. The applyChangeset method was updated to handle these phases, and a wrapOperation helper was added for consistent error handling and timing. Feedback was provided regarding the potential simplification of error handling in applyPlan, as certain logic appears unreachable given that wrapOperation catches all exceptions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Fabricator to implement a two-phase deployment strategy where all function creations and updates are executed before any deletions. If any operation in the first phase fails, the deletion phase is skipped to prevent accidental service gaps. The applyPlan and applyChangeset methods were updated to support this logic, and a new wrapOperation helper was added for consistent error handling and telemetry. Unit tests were updated to verify the sequential execution and the new return structures. I have no feedback to provide.
This pull request resolves a deployment downtime issue in the Cloud Functions release process by refactoring the Fabricator class to implement a globally phased deployment strategy. Previously, function creates, updates, and deletes across all regions were executed in parallel. This behavior caused potential downtime during function renames, as an existing function could be deleted before its replacement was successfully created or updated in another region.
To address this, the execution lifecycle in applyPlan has been split into two distinct global phases separated by a wait barrier. In the first phase, all function creates and updates across all changesets are triggered and awaited simultaneously. In the second phase, deletes are executed only if all operations in the first phase completed successfully. If any create or update operation fails, all pending deletes are aborted, leaving the existing functions running in production and preventing accidental downtime.
The original applyChangeset method was preserved but refactored to accept a phase parameter. This allows it to handle either creations and updates or deletions depending on the current phase of execution directed by applyPlan. Additionally, a reusable wrapOperation helper was extracted to eliminate code duplication across operations and ensure uniform timing and error handling, making the orchestration logic easier to read and maintain.