Skip to content

Commit

Permalink
refactor(igor): Cleanup RetrofitError in GetCommitsTask
Browse files Browse the repository at this point in the history
There are total of 5 API calls happening inside the try block:
1. Igor Service API - scmService.compareCommits(): With the addition of the commit - d430b75, the igorService uses SpinnakerRetrofitErrorHandler.
2. Clouddriver API (OortService) - cloudDriverService.getByAmiId() - This is invoked twice in the block, and oortService.getServerGroupFromCluster(): With the addition of the commit - 84a7106, the oortService uses SpinnakerRetrofitErrorHandler.
3. Front50 API - front50Service.get(): With the addition of the commit - ae24d3f, the front50Service uses SpinnakerRetrofitErrorHandler.

and hence with the above mentioned commits, the catch block with RetrofitError becomes irrelevant.
  • Loading branch information
Pranav-b-7 committed Apr 5, 2024
1 parent d9091d1 commit 5ac2f60
Showing 1 changed file with 0 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import com.netflix.spinnaker.orca.pipeline.model.PipelineTrigger
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError

@Slf4j
@Component
Expand Down Expand Up @@ -122,23 +121,7 @@ class GetCommitsTask implements DiffTask {
}

return TaskResult.builder(ExecutionStatus.SUCCEEDED).context(outputs).build()
} catch (RetrofitError e) {
if (e.kind == RetrofitError.Kind.UNEXPECTED) {
// give up on internal errors
return giveUpOnException(repoInfo, sourceInfo, targetInfo, e)
} else if (e.response?.status == 404) {
// just give up on 404
return handle404(repoInfo, sourceInfo, targetInfo, e)
} else { // retry on other status codes
return retryOnException("retrofit error (${e.message})", repoInfo, sourceInfo, targetInfo, retriesRemaining, e)
}
} catch (SpinnakerHttpException e) {
// Some retrofit objects (e.g. cloudDriverService) use
// SpinnakerRetrofitErrorHandler and so throw Spinnaker*Exception
// exceptions, while other retrofit objects throw RetrofitErrors. One day
// perhaps everything will use SpinnakerRetrofitErrorHandler. Until then,
// duplicate the logic for handling RetrofitError also for
// Spinnaker*Exception.
if (e.getResponseCode() == 404) {
// just give up on 404
return handle404(repoInfo, sourceInfo, targetInfo, e)
Expand All @@ -147,7 +130,6 @@ class GetCommitsTask implements DiffTask {
}
} catch (SpinnakerServerException e) {
// give up on internal errors. Note that this includes network errors
// which the above RetrofitError handling swallows.
return giveUpOnException(repoInfo, sourceInfo, targetInfo, e)
} catch (Exception f) { // retry on everything else
return retryOnException("unexpected exception", repoInfo, sourceInfo, targetInfo, retriesRemaining, f)
Expand Down

0 comments on commit 5ac2f60

Please sign in to comment.