Refactor the Retry Build Service to use a ServiceResponse object
Summary
Currently in the Ci::RetryPipelineService
service is not possible to generate exceptions as
it could interrupt all of the builds as soon as one build throws one
Improvements
The RetryBuildService should use a ServiceResponse object some of the advantages described by Fabio in this comment are:
- consistent experience for the caller of this service
- all errors are returned in the same way. Simplifies API/GraphQL/Controller code.
- failures are explicit (today are implicit or abrupt through exceptions)
- improves black box testing in our specs
Risks
Involved components
- https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/retry_build_service.rb
- https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/services/ee/ci/retry_build_service.rb
Optional: Intended side effects
Some caveats of the refactor are the following
- Specs could be checked against the
response.success?
method - Integration specs might fail as we depend on the
Gitlab::Access::AccessDeniedError
exception if there's no sufficient permissions
proposed solution
def execute(buid)
response = reprocess!(build)
return response if response.error?
new_build = response.payload
check_assignable_runner!(new_build)
return ServiceResponse.error(message: 'build failed', payload: new_build) if new_build.failed?
Gitlab::OptimisticLocking.retry_lock(new_build, name: 'retry_build', &:enqueue)
AfterRequeueJobService.new(project, current_user).execute(build)
::MergeRequests::AddTodoWhenBuildFailsService
.new(project: project, current_user: current_user)
.close(new_build)
ServiceResponse.success(payload: new_build)
end
def reprocess!(build)
raise TypeError unless build.instance_of?(Ci::Build)
unless build.retryable?
return ServiceResponse.error(message: 'build can not be retried', payload: build)
end
unless can?(current_user, :update_build, build)
return ServiceResponse.error(message: 'insufficient permissions to retry the build', payload: build)
end
new_build = clone_build(build)
::Ci::Pipelines::AddJobService.new(build.pipeline).execute!(new_build) do |job|
BulkInsertableAssociations.with_bulk_insert do
job.save!
end
end
build.reset # refresh the data to get new values of `retried` and `processed`.
ServiceResponse.success(payload: new_build)
end
Edited by Fabio Pitino