Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(front50): use SpinnakerRetrofitErrorHandler with Front50Service #4623

Merged
merged 6 commits into from
Apr 16, 2024
Merged

refactor(front50): use SpinnakerRetrofitErrorHandler with Front50Service #4623

merged 6 commits into from
Apr 16, 2024

Commits on Apr 16, 2024

  1. refactor(front50): use SpinnakerRetrofitErrorHandler with Front50Service 

    This PR lays the foundational work for upgrading the retrofit version to 2.x, specifically focusing on refactoring the exception handling for Front50Service.

There are no behaviour changes detected with these code changes and hence no additional tests added to demonstrate the functionalities.
Few existing tests are modified to align it with the new changes.
    @Pranav-b-7
    Pranav-b-7 authored and Pranav-b-7 committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    a44b5bb View commit details
    Browse the repository at this point in the history

  2. refactor(applications): Cleanup RetrofitError in DeleteApplicationTask 

    There are 3 APIs invoked inside the try block:
1. Keel Service API - keelService.deleteDeliveryConfig(): With the addition of the commit - c417922, the keelService uses SpinnakerRetrofitErrorHandler.
2. Front50 Service APIs - front50Service.get() and front50Service.delete(): With the addition of the commit - a44b5bb, the front50 service uses SpinnakerRetrofitErrorHandler.

and hence with the above mentioned commits, the catch block with RetrofitError becomes irrelevant.
    @Pranav-b-7
    Pranav-b-7 authored and Pranav-b-7 committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    df4215d View commit details
    Browse the repository at this point in the history

  3. refactor(igor): Cleanup RetrofitError in GetCommitsTask 

    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 APIs (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 - a44b5bb, the front50Service uses SpinnakerRetrofitErrorHandler.

and hence with the above mentioned commits, the catch block with RetrofitError becomes irrelevant.
    @Pranav-b-7
    Pranav-b-7 authored and Pranav-b-7 committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    7c3fe41 View commit details
    Browse the repository at this point in the history

  4. refactor(test/clouddriver): Cleanup impractical tests in TrafficGuard… 

    …Spec

The API call 'front50Service.get(application)' mocked/stubbed to return null value is not realistic, because if the application is not available , then this API would throw a 404 HTTP error and thats already handled in the same class: https://github.com/spinnaker/orca/blob/7e923cc05af0f66b952245722aa69c667f685c5c/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy#L464.

and hence this test is cleaned up.
    @Pranav-b-7
    Pranav-b-7 authored and Pranav-b-7 committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    97b31f4 View commit details
    Browse the repository at this point in the history

  5. refactor(test/applications): Cleanup impractical tests in UpsertAppli… 

    …cationTaskSpec

The API call 'front50Service.get(application)' mocked/stubbed to return null value is not realistic, because if the application is not present, then this API would throw a 404 HTTP error.

and hence this test is cleaned up.
    @Pranav-b-7
    Pranav-b-7 authored and Pranav-b-7 committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    33f1991 View commit details
    Browse the repository at this point in the history

  6. refactor(test): adjust UpsertApplicationTaskSpec tests to use realist… 

    …ic behavior of Front50Service.get

when an application isn't found.  It doesn't return null in this case, it throws an exception.

Restore previously removed "should create an application in global registries" test since
it was providing useful coverage.
    @dbyron-sf
    dbyron-sf committed Apr 16, 2024
    Configuration menu
    Copy the full SHA
    b9de3fb View commit details
    Browse the repository at this point in the history