From 4c391d983ce7da17fdc253e60e78a935b9ecd57a Mon Sep 17 00:00:00 2001 From: Srikanth Govindarajan Date: Fri, 24 Jan 2025 17:56:50 -0800 Subject: [PATCH] Address comments Signed-off-by: Srikanth Govindarajan --- .../lambda/processor/LambdaProcessorIT.java | 10 +++------- .../lambda/common/util/LambdaRetryStrategy.java | 9 +-------- .../common/client/LambdaClientFactoryTest.java | 5 ----- .../lambda/processor/LambdaProcessorTest.java | 7 ------- .../lambda/utils/CountingHttpClient.java | 1 - .../lambda/utils/LambdaRetryStrategyTest.java | 17 +++-------------- 6 files changed, 7 insertions(+), 42 deletions(-) diff --git a/data-prepper-plugins/aws-lambda/src/integrationTest/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorIT.java b/data-prepper-plugins/aws-lambda/src/integrationTest/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorIT.java index 2e25c053ae..839bcde412 100644 --- a/data-prepper-plugins/aws-lambda/src/integrationTest/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorIT.java +++ b/data-prepper-plugins/aws-lambda/src/integrationTest/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorIT.java @@ -105,12 +105,9 @@ private LambdaProcessor createObjectUnderTest(LambdaProcessorConfig processorCon @BeforeEach public void setup() { -// lambdaRegion = System.getProperty("tests.lambda.processor.region"); -// functionName = System.getProperty("tests.lambda.processor.functionName"); -// role = System.getProperty("tests.lambda.processor.sts_role_arn"); - lambdaRegion = "us-west-2"; - functionName = "lambdaNoReturn"; - role = "arn:aws:iam::176893235612:role/osis-s3-opensearch-role"; + lambdaRegion = System.getProperty("tests.lambda.processor.region"); + functionName = System.getProperty("tests.lambda.processor.functionName"); + role = System.getProperty("tests.lambda.processor.sts_role_arn"); pluginMetrics = mock(PluginMetrics.class); pluginSetting = mock(PluginSetting.class); @@ -456,7 +453,6 @@ void testTooManyRequestsExceptionWithCustomRetryCondition() { // 7) Observe how many total network requests occurred (including SDK retries) int totalRequests = countingHttpClient.getRequestCount(); - System.out.println("Total network requests (including retries): " + totalRequests); // Optionally: If you want to confirm the EXACT number, // this might vary depending on how many parallel calls and how your TMR throttles them. diff --git a/data-prepper-plugins/aws-lambda/src/main/java/org/opensearch/dataprepper/plugins/lambda/common/util/LambdaRetryStrategy.java b/data-prepper-plugins/aws-lambda/src/main/java/org/opensearch/dataprepper/plugins/lambda/common/util/LambdaRetryStrategy.java index ad5536f5f3..6ac43a60c1 100644 --- a/data-prepper-plugins/aws-lambda/src/main/java/org/opensearch/dataprepper/plugins/lambda/common/util/LambdaRetryStrategy.java +++ b/data-prepper-plugins/aws-lambda/src/main/java/org/opensearch/dataprepper/plugins/lambda/common/util/LambdaRetryStrategy.java @@ -1,21 +1,14 @@ package org.opensearch.dataprepper.plugins.lambda.common.util; -import org.opensearch.dataprepper.plugins.lambda.common.accumlator.Buffer; -import org.opensearch.dataprepper.plugins.lambda.common.config.LambdaCommonConfig; import software.amazon.awssdk.core.exception.SdkClientException; -import software.amazon.awssdk.services.lambda.LambdaAsyncClient; -import software.amazon.awssdk.services.lambda.model.InvokeRequest; import software.amazon.awssdk.services.lambda.model.InvokeResponse; import software.amazon.awssdk.services.lambda.model.TooManyRequestsException; import software.amazon.awssdk.services.lambda.model.ServiceException; -import java.time.Duration; import java.util.Arrays; import java.util.HashSet; import java.util.Set; -import org.slf4j.Logger; -import static org.opensearch.dataprepper.plugins.lambda.common.LambdaCommonHandler.isSuccess; /** * Similar to BulkRetryStrategy in the OpenSearch sink. @@ -74,7 +67,7 @@ private LambdaRetryStrategy() { ) ); - public static boolean isRetryable(final int statusCode) { + public static boolean isRetryableStatusCode(final int statusCode) { return TIMEOUT_ERRORS.contains(statusCode) || (statusCode >= 500 && statusCode < 600); } diff --git a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/common/client/LambdaClientFactoryTest.java b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/common/client/LambdaClientFactoryTest.java index 4a85733404..07978bfe85 100644 --- a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/common/client/LambdaClientFactoryTest.java +++ b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/common/client/LambdaClientFactoryTest.java @@ -25,17 +25,12 @@ import software.amazon.awssdk.services.lambda.LambdaAsyncClient; import software.amazon.awssdk.services.lambda.model.InvokeRequest; import software.amazon.awssdk.services.lambda.model.InvokeResponse; -import software.amazon.awssdk.services.lambda.model.TooManyRequestsException; import java.util.HashMap; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicInteger; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.spy; diff --git a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorTest.java b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorTest.java index 3c63aaaf81..902efcf739 100644 --- a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorTest.java +++ b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/processor/LambdaProcessorTest.java @@ -30,18 +30,14 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.mockito.MockitoAnnotations; -import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; -import org.mockito.stubbing.Answer; import org.opensearch.dataprepper.aws.api.AwsCredentialsSupplier; import org.opensearch.dataprepper.expression.ExpressionEvaluator; import org.opensearch.dataprepper.model.acknowledgements.AcknowledgementSet; @@ -56,12 +52,10 @@ import org.opensearch.dataprepper.plugins.codec.json.JsonInputCodecConfig; import org.opensearch.dataprepper.plugins.lambda.common.accumlator.Buffer; import org.opensearch.dataprepper.plugins.lambda.common.accumlator.InMemoryBuffer; -import org.opensearch.dataprepper.plugins.lambda.common.client.LambdaClientFactory; import org.opensearch.dataprepper.plugins.lambda.common.config.AwsAuthenticationOptions; import org.opensearch.dataprepper.plugins.lambda.common.config.BatchOptions; import org.opensearch.dataprepper.plugins.lambda.common.config.ClientOptions; import org.opensearch.dataprepper.plugins.lambda.common.config.InvocationType; -import org.opensearch.dataprepper.plugins.lambda.common.util.CustomLambdaRetryCondition; import org.opensearch.dataprepper.plugins.lambda.processor.exception.StrictResponseModeNotRespectedException; import static org.opensearch.dataprepper.plugins.lambda.utils.LambdaTestSetupUtil.createLambdaConfigurationFromYaml; import static org.opensearch.dataprepper.plugins.lambda.utils.LambdaTestSetupUtil.getSampleEventRecords; @@ -71,7 +65,6 @@ import software.amazon.awssdk.services.lambda.LambdaAsyncClient; import software.amazon.awssdk.services.lambda.model.InvokeRequest; import software.amazon.awssdk.services.lambda.model.InvokeResponse; -import software.amazon.awssdk.services.lambda.model.TooManyRequestsException; import java.io.IOException; import java.io.InputStream; diff --git a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/CountingHttpClient.java b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/CountingHttpClient.java index 8fc58feccf..feddd99538 100644 --- a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/CountingHttpClient.java +++ b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/CountingHttpClient.java @@ -2,7 +2,6 @@ import software.amazon.awssdk.http.async.SdkAsyncHttpClient; import software.amazon.awssdk.http.async.AsyncExecuteRequest; -import software.amazon.awssdk.utils.CompletableFutureUtils; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; diff --git a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/LambdaRetryStrategyTest.java b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/LambdaRetryStrategyTest.java index 74c73baf92..064b24d8fc 100644 --- a/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/LambdaRetryStrategyTest.java +++ b/data-prepper-plugins/aws-lambda/src/test/java/org/opensearch/dataprepper/plugins/lambda/utils/LambdaRetryStrategyTest.java @@ -13,25 +13,14 @@ import org.opensearch.dataprepper.plugins.lambda.common.config.LambdaCommonConfig; import org.opensearch.dataprepper.plugins.lambda.common.util.LambdaRetryStrategy; import org.slf4j.Logger; -import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.services.lambda.LambdaAsyncClient; -import software.amazon.awssdk.services.lambda.model.InvokeRequest; import software.amazon.awssdk.services.lambda.model.InvokeResponse; -import software.amazon.awssdk.services.lambda.model.ServiceException; -import software.amazon.awssdk.services.lambda.model.TooManyRequestsException; import java.time.Duration; -import java.util.concurrent.CompletableFuture; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -62,9 +51,9 @@ void setUp() { @Test void testIsRetryable() { - assertTrue(LambdaRetryStrategy.isRetryable(429)); - assertTrue(LambdaRetryStrategy.isRetryable(500)); - assertFalse(LambdaRetryStrategy.isRetryable(200)); + assertTrue(LambdaRetryStrategy.isRetryableStatusCode(429)); + assertTrue(LambdaRetryStrategy.isRetryableStatusCode(500)); + assertFalse(LambdaRetryStrategy.isRetryableStatusCode(200)); } @Test