Skip to content

Conversation

@ericsciple
Copy link
Collaborator

@ericsciple ericsciple commented Apr 11, 2025

@ericsciple ericsciple marked this pull request as ready for review April 11, 2025 05:03
Copilot AI review requested due to automatic review settings April 11, 2025 05:03
@ericsciple ericsciple requested a review from a team as a code owner April 11, 2025 05:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes legacy feature flag support for "actions_skip_retry_complete_job_upon_known_errors" to simplify job completion logic. Key changes include:

  • Removing the conditional branch that calls CompleteJob2Async and its associated catch filters in JobRunner.cs.
  • Eliminating the CompleteJob2Async method from the RunServer interface in RunServer.cs.
  • Deleting the SkipRetryCompleteJobUponKnownErrors constant from Constants.cs.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Runner.Worker/JobRunner.cs Removed conditional logic based on the feature flag and updated exception handling.
src/Runner.Common/RunServer.cs Removed legacy CompleteJob2Async method and related implementation.
src/Runner.Common/Constants.cs Removed the deprecated feature flag constant.
Comments suppressed due to low confidence (2)

src/Runner.Common/RunServer.cs:32

  • Ensure that all references to 'CompleteJob2Async' have been completely removed after deprecation to avoid any unintentional calls or interface mismatches in the future.
Task CompleteJob2Async(

src/Runner.Worker/JobRunner.cs:324

  • The removal of the feature flag check in the catch block changes the exception handling behavior. Please confirm that this uniform approach to handling VssUnauthorizedException is intentional and does not hide side effects previously gated by the flag.
catch (VssUnauthorizedException ex)

@ericsciple ericsciple merged commit 5106d65 into main Apr 11, 2025
9 checks passed
@ericsciple ericsciple deleted the users/ericsciple/25-04-clean branch April 11, 2025 13:34
sirredbeard pushed a commit to sirredbeard/runner that referenced this pull request Jun 11, 2025
sirredbeard pushed a commit to sirredbeard/runner that referenced this pull request Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants