Skip to content

Commit

Permalink
Merge branch 'develop' into ps/dt-1184-sql-retry-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
pshapiro4broad committed Jan 30, 2025
2 parents d897097 + 3ec1247 commit f9f8adb
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 5 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ plugins {

allprojects {
group 'bio.terra'
version '2.217.0-SNAPSHOT'
version '2.219.0-SNAPSHOT'

ext {
resourceDir = "${rootDir}/src/main/resources/api"
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/bio/terra/common/AclUtils.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package bio.terra.common;

import bio.terra.service.resourcemanagement.exception.BigQueryAclExhaustionException;
import bio.terra.service.resourcemanagement.exception.GoogleResourceException;
import bio.terra.service.resourcemanagement.exception.UpdatePermissionsFailedException;
import com.google.cloud.bigquery.BigQueryException;
import java.util.Random;
import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -31,6 +33,17 @@ public static <T> T aclUpdateRetry(Callable<T> aclUpdate) throws InterruptedExce
ex);
lastException = ex.getCause();
} catch (Exception ex) {
// If an exception is thrown due to the user exhausting the number of authorized entities
// in the BigQuery dataset, detect that case and return a custom exception so it can be
// better reported to the user.
if (ex instanceof BigQueryException bqe
&& bqe.getMessage().startsWith("Too many authorized entities in this dataset.")) {
throw new BigQueryAclExhaustionException(
bqe.getMessage()
+ " Resolve this by deleting snapshots or creating a second Terra Data Repo dataset.",
bqe);
}

throw new GoogleResourceException("Error while performing ACL update", ex);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package bio.terra.service.resourcemanagement.exception;

import bio.terra.common.exception.BadRequestException;

public class BigQueryAclExhaustionException extends BadRequestException {
// This constructor is required so this can be deserialized using StairwayExceptionSerializer.
public BigQueryAclExhaustionException(String message) {
super(message);
}

public BigQueryAclExhaustionException(String message, Throwable cause) {
super(message, cause);
}
}
26 changes: 26 additions & 0 deletions src/test/java/bio/terra/common/AclUtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package bio.terra.common;

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

import bio.terra.common.category.Unit;
import bio.terra.service.resourcemanagement.exception.BigQueryAclExhaustionException;
import com.google.cloud.bigquery.BigQueryException;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

@Tag(Unit.TAG)
class AclUtilsTest {

@Test
void aclUpdateRetry() {
assertThrows(
BigQueryAclExhaustionException.class,
() ->
AclUtils.aclUpdateRetry(
() -> {
throw new BigQueryException(
0,
"Too many authorized entities in this dataset. The maximum number of authorized views, routines, and datasets combined is 2500.");
}));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import bio.terra.service.resourcemanagement.google.GoogleResourceManagerService;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.opentelemetry.api.OpenTelemetry;
import java.io.IOException;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -53,10 +54,10 @@
public class IntegrationTestConfiguration {

@Bean("tdrServiceAccountEmail")
public String tdrServiceAccountEmail() {
// Provide a default value for the service account email when running a spring-context aware
// test to avoid having to set it in the test environment.
return "";
public String tdrServiceAccountEmail() throws IOException {
// Delegate to ApplicationConfiguration to get the service account email. Without this set,
// the service account would be deleted by tests on teardown.
return new ApplicationConfiguration().tdrServiceAccountEmail();
}

@Bean
Expand Down

0 comments on commit f9f8adb

Please sign in to comment.