Skip to content

Commit

Permalink
Ignore 429s when validating a connection (#83)
Browse files Browse the repository at this point in the history
  • Loading branch information
aymeric-dispa authored Sep 22, 2022
1 parent bcb0949 commit 343e562
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 14 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/deploy-javadoc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ name: Deploy Javadoc
on:
workflow_dispatch:
release:
types: [published]
types: [ published ]
jobs:
publish:
if: ${{ !contains(github.event.release.name, 'SNAPSHOT') }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
10 changes: 5 additions & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ plugins {
}

group 'com.firebolt'
version = '2.1.1-SNAPSHOT'
version = '2.1.2-SNAPSHOT'

repositories {
mavenCentral()
Expand Down Expand Up @@ -55,7 +55,7 @@ sourceSets {
dependencies {
implementation 'org.apache.commons:commons-lang3:3.12.0'
implementation 'com.google.guava:guava:31.1-jre'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.3'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.4'
implementation 'commons-codec:commons-codec:1.15'
implementation 'net.jodah:expiringmap:0.5.10'
implementation 'org.apache.httpcomponents.client5:httpclient5:5.1.3'
Expand All @@ -73,9 +73,9 @@ dependencies {
testCompileOnly 'org.projectlombok:lombok:1.18.24'
testAnnotationProcessor 'org.projectlombok:lombok:1.18.24'

testImplementation 'org.mockito:mockito-junit-jupiter:4.7.0'
testImplementation 'org.mockito:mockito-core:4.7.0'
testImplementation 'org.mockito:mockito-inline:4.7.0'
testImplementation 'org.mockito:mockito-junit-jupiter:4.8.0'
testImplementation 'org.mockito:mockito-core:4.8.0'
testImplementation 'org.mockito:mockito-inline:4.8.0'
testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.0'
testImplementation 'org.junit.jupiter:junit-jupiter-engine:5.9.0'
testImplementation group: 'org.junit-pioneer', name: 'junit-pioneer', version: '1.7.1'
Expand Down
19 changes: 13 additions & 6 deletions src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.firebolt.jdbc.client.authentication.FireboltAuthenticationClient;
import com.firebolt.jdbc.client.query.StatementClientImpl;
import com.firebolt.jdbc.connection.settings.FireboltProperties;
import com.firebolt.jdbc.exception.ExceptionType;
import com.firebolt.jdbc.exception.FireboltException;
import com.firebolt.jdbc.exception.FireboltSQLFeatureNotSupportedException;
import com.firebolt.jdbc.exception.FireboltUnsupportedOperationException;
Expand Down Expand Up @@ -333,19 +334,25 @@ public boolean isValid(int timeout) throws SQLException {
return false;
}
try {
validateConnection(this.getSessionProperties());
validateConnection(this.getSessionProperties(), true);
return true;
} catch (Exception e) {
return false;
}
}

private void validateConnection(FireboltProperties fireboltProperties) throws SQLException {
private void validateConnection(FireboltProperties fireboltProperties, boolean ignoreToManyRequestsError) throws SQLException {
try (Statement s = createStatement(fireboltProperties)) {
s.execute("SELECT 1");
} catch (Exception e) {
log.warn("Connection is not valid", e);
throw e;
} catch (FireboltException e) {
//A connection is not invalid when too many requests are being sent.
//This error cannot be ignored when testing the connection to validate a param.
if (e.getType() == ExceptionType.TOO_MANY_REQUESTS && ignoreToManyRequestsError) {
log.warn("Too many requests are sent to the server", e);
} else {
log.warn("Connection is not valid", e);
throw e;
}
}
}

Expand All @@ -365,7 +372,7 @@ public synchronized void addProperty(Pair<String, String> property) throws Fireb
try {
FireboltProperties tmpProperties = FireboltProperties.copy(this.sessionProperties);
tmpProperties.addProperty(property);
validateConnection(tmpProperties);
validateConnection(tmpProperties, false);
this.sessionProperties.addProperty(property);
} catch (FireboltException e) {
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
*/
public enum ExceptionType {
ERROR, EXPIRED_TOKEN, TYPE_NOT_SUPPORTED, TYPE_TRANSFORMATION_ERROR, RESOURCE_NOT_FOUND, REQUEST_FAILED,
INVALID_REQUEST
INVALID_REQUEST, TOO_MANY_REQUESTS
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ private ExceptionType getExceptionType(Integer httpStatusCode) {
return INVALID_REQUEST;
case SC_UNAUTHORIZED:
return EXPIRED_TOKEN;
case SC_TOO_MANY_REQUESTS:
return TOO_MANY_REQUESTS;
default:
return ERROR;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.mockito.junit.jupiter.MockitoExtension;

import com.firebolt.jdbc.connection.settings.FireboltProperties;
import com.firebolt.jdbc.exception.ExceptionType;
import com.firebolt.jdbc.exception.FireboltException;
import com.firebolt.jdbc.service.FireboltAuthenticationService;
import com.firebolt.jdbc.service.FireboltEngineService;
Expand Down Expand Up @@ -108,7 +109,7 @@ void shouldNotSetNewPropertyWhenConnectionIsNotValidWithTheNewProperty() throws
fireboltAuthenticationService, fireboltEngineService, fireboltStatementService);

when(fireboltStatementService.execute(any(), any(), any()))
.thenThrow(new IllegalArgumentException("The property is invalid"));
.thenThrow(new FireboltException(ExceptionType.TOO_MANY_REQUESTS));
assertThrows(FireboltException.class,
() -> fireboltConnection.addProperty(new ImmutablePair<>("custom_1", "1")));

Expand Down Expand Up @@ -148,6 +149,24 @@ void shouldValidateConnectionWhenCallingIsValid() throws SQLException {
assertEquals("SELECT 1", queryInfoWrapperArgumentCaptor.getValue().getSql());
}

@Test
void shouldIgnore429WhenValidatingConnection() throws SQLException {
when(fireboltStatementService.execute(any(), any(), any()))
.thenThrow(new FireboltException(ExceptionType.TOO_MANY_REQUESTS));
FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties,
fireboltAuthenticationService, fireboltEngineService, fireboltStatementService);
assertTrue(fireboltConnection.isValid(500));
}

@Test
void shouldReturnFalseWhenValidatingConnectionThrowsAnException() throws SQLException {
when(fireboltStatementService.execute(any(), any(), any()))
.thenThrow(new FireboltException(ExceptionType.ERROR));
FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties,
fireboltAuthenticationService, fireboltEngineService, fireboltStatementService);
assertFalse(fireboltConnection.isValid(500));
}

@Test
void shouldThrowExceptionWhenValidatingConnectionWithNegativeTimeout() throws SQLException {
FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties,
Expand Down

0 comments on commit 343e562

Please sign in to comment.