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.

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

TSPS-138 add cbas client integration #49

Merged
merged 2 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion service/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ dependencies {
// terra clients
implementation "org.broadinstitute.dsde.workbench:sam-client_2.13:0.1-fd8ee25"
implementation "org.broadinstitute.dsde.workbench:leonardo-client_2.13:1.3.6-66d9fcf"
implementation 'org.databiosphere:workspacedataservice-client-okhttp-javax:0.2.104-SNAPSHOT'
implementation "org.databiosphere:workspacedataservice-client-okhttp-javax:0.2.104-SNAPSHOT"
implementation "bio.terra:cbas-client:0.0.183"

liquibaseRuntime 'org.liquibase:liquibase-core:3.10.0'
liquibaseRuntime 'info.picocli:picocli:4.6.1'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package bio.terra.pipelines.app.configuration.external;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "cbas")
public record CbasConfiguration(Boolean debugApiLogging) {}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public RetryTemplate listenerResetRetryTemplate() {
FixedBackOffPolicy fixedBackOffPolicy = new FixedBackOffPolicy();
fixedBackOffPolicy.setBackOffPeriod(1000L);

// Inner retry (assumping the classifier hits): up to 3 times
// Inner retry (assuming the classifier hits): up to 3 times
SimpleRetryPolicy srp = new SimpleRetryPolicy();
srp.setMaxAttempts(3);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package bio.terra.pipelines.dependencies.cbas;

import bio.terra.cbas.api.MethodsApi;
import bio.terra.cbas.api.PublicApi;
import bio.terra.cbas.api.RunSetsApi;
import bio.terra.cbas.api.RunsApi;
import bio.terra.cbas.client.ApiClient;
import bio.terra.pipelines.app.configuration.external.CbasConfiguration;
import org.springframework.stereotype.Component;

@Component
public class CbasClient {

private final CbasConfiguration cbasConfiguration;

public CbasClient(CbasConfiguration cbasConfiguration) {
this.cbasConfiguration = cbasConfiguration;
}

protected ApiClient getApiClient(String cbasBaseUri, String accessToken) {
ApiClient apiClient = new ApiClient().setBasePath(cbasBaseUri);
apiClient.addDefaultHeader("Authorization", "Bearer " + accessToken);
// By closing the connection after each request, we avoid the problem of the open connection
// being force-closed ungracefully by the Azure Relay/Listener infrastructure:
apiClient.addDefaultHeader("Connection", "close");
apiClient.setDebugging(cbasConfiguration.debugApiLogging());
return apiClient;
}

// used to access public endpoints like status and health
PublicApi publicApi(String cbasBaseUri, String accessToken) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it'd be helpful to have a brief comment for each of these describing what functionality each is used for

return new PublicApi(getApiClient(cbasBaseUri, accessToken));
}

// used to access endpoints related to methods (wdls)
// create, search, delete,etc.
MethodsApi methodsApi(String cbasBaseUri, String accessToken) {
return new MethodsApi(getApiClient(cbasBaseUri, accessToken));
}

// used to access endpoints to get information related to runs in a run set
RunsApi runsApi(String cbasBaseUri, String accessToken) {
return new RunsApi(getApiClient(cbasBaseUri, accessToken));
}

// used to access endpoints related to running of a submission of methods
// launch, abort, list run sets for a given method
RunSetsApi runSetsApi(String cbasBaseUri, String accessToken) {
return new RunSetsApi(getApiClient(cbasBaseUri, accessToken));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package bio.terra.pipelines.dependencies.cbas;

import bio.terra.cbas.client.ApiException;
import bio.terra.cbas.model.MethodListResponse;
import bio.terra.cbas.model.SystemStatus;
import bio.terra.pipelines.dependencies.common.HealthCheckWorkspaceApps;
import org.springframework.retry.support.RetryTemplate;
import org.springframework.stereotype.Service;

@Service
public class CbasService implements HealthCheckWorkspaceApps {
private final CbasClient cbasClient;
private final RetryTemplate listenerResetRetryTemplate;

public CbasService(CbasClient cbasClient, RetryTemplate listenerResetRetryTemplate) {
this.cbasClient = cbasClient;
this.listenerResetRetryTemplate = listenerResetRetryTemplate;
}

public MethodListResponse getAllMethods(String cbasBaseUri, String accesstoken) {
return executionWithRetryTemplate(
listenerResetRetryTemplate,
() -> cbasClient.methodsApi(cbasBaseUri, accesstoken).getMethods(null, null, null));
}

@Override
public Result checkHealth(String wdsBaseUri, String accessToken) {
try {
SystemStatus result = cbasClient.publicApi(wdsBaseUri, accessToken).getStatus();
return new Result(result.isOk(), result.toString());
} catch (ApiException e) {
return new Result(false, e.getMessage());
}
}

interface CbasAction<T> {
T execute() throws ApiException;
}

static <T> T executionWithRetryTemplate(
RetryTemplate retryTemplate, CbasService.CbasAction<T> action)
throws CbasServiceApiException {

return retryTemplate.execute(
context -> {
try {
return action.execute();
} catch (ApiException e) {
throw new CbasServiceApiException(e);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package bio.terra.pipelines.dependencies.cbas;

import bio.terra.cbas.client.ApiException;

public class CbasServiceApiException extends CbasServiceException {

public CbasServiceApiException(ApiException exception) {
super("Cbas returned an unsuccessful status code", exception);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package bio.terra.pipelines.dependencies.cbas;

import bio.terra.common.exception.ErrorReportException;
import java.util.ArrayList;
import org.springframework.http.HttpStatus;

public class CbasServiceException extends ErrorReportException {
protected CbasServiceException(String message, Throwable cause) {
super(message, cause, new ArrayList<>(), HttpStatus.INTERNAL_SERVER_ERROR);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
public class LeonardoService implements HealthCheck {

private final LeonardoClient leonardoClient;

private final AppUtils appUtils;
private final RetryTemplate listenerResetRetryTemplate;

Expand Down Expand Up @@ -49,6 +48,10 @@ public String getWdsUrlFromApps(String workspaceId, String authToken, boolean cr
return appUtils.findUrlForWds(getApps(workspaceId, authToken, creatorOnly), workspaceId);
}

public String getCbasUrlFromApps(String workspaceId, String authToken, boolean creatorOnly) {
return appUtils.findUrlForCbas(getApps(workspaceId, authToken, creatorOnly), workspaceId);
}

@Override
public Result checkHealth() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import bio.terra.pipelines.db.exception.DuplicateObjectException;
import bio.terra.pipelines.db.repositories.ImputationJobsRepository;
import bio.terra.pipelines.db.repositories.PipelineInputsRepository;
import bio.terra.pipelines.dependencies.cbas.CbasService;
import bio.terra.pipelines.dependencies.cbas.CbasServiceException;
import bio.terra.pipelines.dependencies.leonardo.LeonardoService;
import bio.terra.pipelines.dependencies.leonardo.LeonardoServiceException;
import bio.terra.pipelines.dependencies.sam.SamService;
Expand Down Expand Up @@ -38,6 +40,7 @@ public class ImputationService {
private LeonardoService leonardoService;
private SamService samService;
private WdsService wdsService;
private CbasService cbasService;
private final JobService jobService;
private ImputationConfiguration imputationConfiguration;

Expand All @@ -48,13 +51,15 @@ public class ImputationService {
LeonardoService leonardoService,
SamService samService,
WdsService wdsService,
CbasService cbasService,
JobService jobService,
ImputationConfiguration imputationConfiguration) {
this.imputationJobsRepository = imputationJobsRepository;
this.pipelineInputsRepository = pipelineInputsRepository;
this.leonardoService = leonardoService;
this.samService = samService;
this.wdsService = wdsService;
this.cbasService = cbasService;
this.jobService = jobService;
this.imputationConfiguration = imputationConfiguration;
}
Expand Down Expand Up @@ -118,6 +123,7 @@ public UUID writeJobToDb(
public List<ListAppResponse> queryForWorkspaceApps() {
String workspaceId = imputationConfiguration.workspaceId();
try {
// leonardo related calls
List<ListAppResponse> getAppsResponse =
leonardoService.getApps(workspaceId, samService.getTspsServiceAccountToken(), false);

Expand All @@ -126,6 +132,7 @@ public List<ListAppResponse> queryForWorkspaceApps() {
imputationConfiguration.workspaceId(),
getAppsResponse);

// wds related calls
String wdsUri =
leonardoService.getWdsUrlFromApps(
workspaceId, samService.getTspsServiceAccountToken(), false);
Expand All @@ -141,13 +148,29 @@ public List<ListAppResponse> queryForWorkspaceApps() {
samService.getTspsServiceAccountToken(),
imputationConfiguration.workspaceId()));

// cbas related calls
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to remind myself, this whole method queryForWorkspaceApps() is just a placeholder to make these calls and show they're working, right? eventually these will be moved into flight steps and the uris will be used to make calls etc etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup yup exactly

String cbasUri =
leonardoService.getCbasUrlFromApps(
workspaceId, samService.getTspsServiceAccountToken(), false);
logger.info(
"cbas uri for workspace id {}: {}", imputationConfiguration.workspaceId(), cbasUri);
logger.info(
"Cbas health: {}",
cbasService.checkHealth(cbasUri, samService.getTspsServiceAccountToken()));
logger.info(
"list of methods available: {}",
cbasService.getAllMethods(cbasUri, samService.getTspsServiceAccountToken()));

return getAppsResponse;
} catch (LeonardoServiceException e) {
logger.error("Get Apps called for workspace id {} failed", workspaceId);
return Collections.emptyList();
} catch (WdsServiceException e) {
logger.error("Calls to Wds for workspace id {} failed", workspaceId);
return Collections.emptyList();
} catch (CbasServiceException e) {
logger.error("Calls to Cbas for workspace id {} failed", workspaceId);
return Collections.emptyList();
}
}

Expand Down
5 changes: 4 additions & 1 deletion service/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,13 @@ leonardo:
# The below appTypeNames are in priority order for the named app type. This order is necessary for
# the appComparisonFunction() located in AppUtils.java for selecting the best app for each app type.
wdsAppTypeNames: ['WDS']
cbasAppTypeNames: ['CROMWELL']
cbasAppTypeNames: ['WORKFLOWS_APP', 'CROMWELL']
dependencyUrlCacheTtlSeconds: 300 # Refresh every 5 minutes
debugApiLogging: false

wds:
apiV: "v0.2"
debugApiLogging: false

cbas:
debugApiLogging: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package bio.terra.pipelines.configuration.external;

import static org.junit.jupiter.api.Assertions.assertTrue;

import bio.terra.pipelines.app.configuration.external.CbasConfiguration;
import bio.terra.pipelines.testutils.BaseEmbeddedDbTest;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

class CbasConfigurationTest extends BaseEmbeddedDbTest {
/** test reading Cbas config from application yml */
@Autowired CbasConfiguration cbasConfiguration;

@Test
void verifyCbasServerConfig() {
assertTrue(cbasConfiguration.debugApiLogging());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class SamConfigurationTest extends BaseEmbeddedDbTest {
@Autowired SamConfiguration samConfiguration;

@Test
void verifyLeonardoServerConfig() {
void verifySamServerConfig() {
assertEquals("testSamUri", samConfiguration.baseUri());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package bio.terra.pipelines.dependencies.cbas;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import bio.terra.cbas.api.MethodsApi;
import bio.terra.cbas.api.PublicApi;
import bio.terra.cbas.api.RunSetsApi;
import bio.terra.cbas.api.RunsApi;
import bio.terra.pipelines.testutils.BaseEmbeddedDbTest;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

class CbasClientTest extends BaseEmbeddedDbTest {
@Autowired CbasClient cbasClient;
String cbasBaseUri = "cbasBaseUri";
String authToken = "authToken";

@Test
void TestWdsClientApis() {

PublicApi publicApi = cbasClient.publicApi(cbasBaseUri, authToken);

assertEquals(cbasBaseUri, publicApi.getApiClient().getBasePath());
assertTrue(publicApi.getApiClient().isDebugging());
Copy link
Collaborator

Choose a reason for hiding this comment

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

what sets this to True? is that the default that the CBAS client sets? I guess what I'm really asking is, why are we testing this?

Copy link
Collaborator Author

@jsotobroad jsotobroad Jan 22, 2024

Choose a reason for hiding this comment

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

its set to true in the application-test.yml file but realistically there isnt any reason to test this simple of a config. Pretty much just wrote it cuz it was easy and if we ever decide to something more complicated (like the leonardo config) we'll have this set up already


MethodsApi methodsApi = cbasClient.methodsApi(cbasBaseUri, authToken);

assertEquals(cbasBaseUri, methodsApi.getApiClient().getBasePath());
assertTrue(methodsApi.getApiClient().isDebugging());

RunsApi runsApi = cbasClient.runsApi(cbasBaseUri, authToken);

assertEquals(cbasBaseUri, runsApi.getApiClient().getBasePath());
assertTrue(runsApi.getApiClient().isDebugging());

RunSetsApi runSetsApi = cbasClient.runSetsApi(cbasBaseUri, authToken);

assertEquals(cbasBaseUri, runSetsApi.getApiClient().getBasePath());
assertTrue(runSetsApi.getApiClient().isDebugging());
}
}
Loading
Loading