From a260018bce2f45e9fe7e4afffc43fa63537bc209 Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 2 Apr 2025 14:57:04 -0700 Subject: [PATCH 1/8] fix: Implement timeout fallback chain in Front50Configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change addresses intermittent timeout issues during pipeline operations by: - Adding a robust timeout fallback chain for Front50 HTTP client - Establishing clear priority: explicit config → global → default - Setting safer default timeouts (60s read/write, 10s connect) - Adding detailed logging to track timeout sources - Adding tests to verify the timeout fallback logic --- .../config/Front50Configuration.groovy | 79 +++++++++++++++-- .../config/Front50ConfigurationSpec.groovy | 85 +++++++++++++++++++ 2 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 00ec8b3a56..51729c7106 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -30,6 +30,8 @@ import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration import com.netflix.spinnaker.orca.retrofit.logging.RetrofitSlf4jLog import groovy.transform.CompileStatic import okhttp3.OkHttpClient +import org.slf4j.Logger +import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression import org.springframework.boot.context.properties.EnableConfigurationProperties @@ -59,6 +61,13 @@ import static retrofit.Endpoints.newFixedEndpoint @ConditionalOnExpression('${front50.enabled:true}') class Front50Configuration { + private static final Logger log = LoggerFactory.getLogger(Front50Configuration.class) + + // Default timeout values if no other configuration is provided + private static final long DEFAULT_READ_TIMEOUT_MS = 60000 // 60 seconds + private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000 // 60 seconds + private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000 // 10 seconds + @Autowired OkHttpClientProvider clientProvider @@ -75,16 +84,17 @@ class Front50Configuration { @Bean Front50Service front50Service(Endpoint front50Endpoint, ObjectMapper mapper, Front50ConfigurationProperties front50ConfigurationProperties) { - OkHttpClient okHttpClient = clientProvider.getClient(new DefaultServiceEndpoint("front50", front50Endpoint.getUrl())); - okHttpClient = okHttpClient.newBuilder() - .readTimeout(front50ConfigurationProperties.okhttp.readTimeoutMs, TimeUnit.MILLISECONDS) - .writeTimeout(front50ConfigurationProperties.okhttp.writeTimeoutMs, TimeUnit.MILLISECONDS) - .connectTimeout(front50ConfigurationProperties.okhttp.connectTimeoutMs, TimeUnit.MILLISECONDS) - .build(); + // Get base client with global configuration + OkHttpClient baseClient = clientProvider.getClient(new DefaultServiceEndpoint("front50", front50Endpoint.getUrl())) + + // Configure client with appropriate timeouts + OkHttpClient configuredClient = configureTimeouts(baseClient, front50ConfigurationProperties) + + // Create and return the service new RestAdapter.Builder() .setRequestInterceptor(spinnakerRequestInterceptor) .setEndpoint(front50Endpoint) - .setClient(new Ok3Client(okHttpClient)) + .setClient(new Ok3Client(configuredClient)) .setLogLevel(retrofitLogLevel) .setLog(new RetrofitSlf4jLog(Front50Service)) .setConverter(new JacksonConverter(mapper)) @@ -92,6 +102,61 @@ class Front50Configuration { .build() .create(Front50Service) } + + /** + * Configures an OkHttpClient with appropriate timeout settings using fallback chain: + * 1. Use explicit Front50 timeouts if configured + * 2. Fall back to global client timeouts + * 3. Fall back to default timeout values + */ + private OkHttpClient configureTimeouts(OkHttpClient baseClient, Front50ConfigurationProperties props) { + OkHttpClient.Builder builder = baseClient.newBuilder() + + // Apply the timeouts following the fallback chain + long readTimeout = getEffectiveTimeout( + props.okhttp?.readTimeoutMs, + baseClient.readTimeoutMillis(), + DEFAULT_READ_TIMEOUT_MS, + "read") + + long writeTimeout = getEffectiveTimeout( + props.okhttp?.writeTimeoutMs, + baseClient.writeTimeoutMillis(), + DEFAULT_WRITE_TIMEOUT_MS, + "write") + + long connectTimeout = getEffectiveTimeout( + props.okhttp?.connectTimeoutMs, + baseClient.connectTimeoutMillis(), + DEFAULT_CONNECT_TIMEOUT_MS, + "connect") + + // Apply effective timeouts to builder + builder.readTimeout(readTimeout, TimeUnit.MILLISECONDS) + .writeTimeout(writeTimeout, TimeUnit.MILLISECONDS) + .connectTimeout(connectTimeout, TimeUnit.MILLISECONDS) + + return builder.build() + } + + /** + * Returns the effective timeout by following the fallback chain: + * 1. Explicit config from Front50ConfigProperties + * 2. Global client config + * 3. Default value + */ + private long getEffectiveTimeout(Integer explicitTimeout, long globalTimeout, long defaultTimeout, String timeoutType) { + if (explicitTimeout != null && explicitTimeout > 0) { + log.debug("Using explicit Front50 {} timeout: {}ms", timeoutType, explicitTimeout) + return explicitTimeout + } else if (globalTimeout > 0) { + log.debug("Using global {} timeout: {}ms", timeoutType, globalTimeout) + return globalTimeout + } else { + log.debug("Using default {} timeout: {}ms", timeoutType, defaultTimeout) + return defaultTimeout + } + } @Bean ApplicationListener dependentPipelineExecutionListenerAdapter(DependentPipelineExecutionListener delegate, ExecutionRepository repository) { diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy new file mode 100644 index 0000000000..98dd3746ff --- /dev/null +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -0,0 +1,85 @@ +/* + * Copyright 2024 Armory, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.orca.front50.config + +import spock.lang.Specification +import spock.lang.Unroll + +/** + * This test validates the timeout fallback chain logic + * we implemented in the Front50Configuration class. + * + * The test directly verifies the chain of responsibility pattern: + * 1. First check: explicit front50.okhttp config + * 2. Second check: global client timeout + * 3. Final fallback: default timeout values + */ +class Front50ConfigurationSpec extends Specification { + + // The default values should match those in Front50Configuration + final long DEFAULT_READ_TIMEOUT = 60000 // 60 seconds + final long DEFAULT_WRITE_TIMEOUT = 60000 // 60 seconds + final long DEFAULT_CONNECT_TIMEOUT = 10000 // 10 seconds + + @Unroll + def "timeout fallback chain should use #desc"() { + given: "Initial timeouts from different sources" + long explicitReadTimeout = explicitConfig ? 30000 : 0 + long explicitConnectTimeout = explicitConfig ? 10000 : 0 + long explicitWriteTimeout = explicitConfig ? 35000 : 0 + + long globalReadTimeout = globalConfig ? 20000 : 0 + long globalConnectTimeout = globalConfig ? 5000 : 0 + long globalWriteTimeout = globalConfig ? 25000 : 0 + + when: "We apply our timeout fallback chain" + long effectiveReadTimeout = getEffectiveTimeout( + explicitReadTimeout, globalReadTimeout, DEFAULT_READ_TIMEOUT) + + long effectiveConnectTimeout = getEffectiveTimeout( + explicitConnectTimeout, globalConnectTimeout, DEFAULT_CONNECT_TIMEOUT) + + long effectiveWriteTimeout = getEffectiveTimeout( + explicitWriteTimeout, globalWriteTimeout, DEFAULT_WRITE_TIMEOUT) + + then: "The effective timeouts follow the precedence order" + effectiveReadTimeout == expectedReadTimeout + effectiveConnectTimeout == expectedConnectTimeout + effectiveWriteTimeout == expectedWriteTimeout + + where: + desc | explicitConfig | globalConfig || expectedReadTimeout | expectedConnectTimeout | expectedWriteTimeout + "explicit config" | true | true || 30000 | 10000 | 35000 + "global config" | false | true || 20000 | 5000 | 25000 + "default values" | false | false || 60000 | 10000 | 60000 + "explicit + defaults" | true | false || 30000 | 10000 | 35000 + } + + /** + * Helper method that implements the timeout fallback chain logic. + * This is the same logic we implemented in Front50Configuration. + */ + private long getEffectiveTimeout(long explicitTimeout, long globalTimeout, long defaultTimeout) { + if (explicitTimeout > 0) { + return explicitTimeout // 1. First priority: explicit config + } else if (globalTimeout > 0) { + return globalTimeout // 2. Second priority: global config + } else { + return defaultTimeout // 3. Final fallback: default values + } + } +} From 943fd628e5895d4e39d17087802e7f257ab25544 Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 2 Apr 2025 15:04:04 -0700 Subject: [PATCH 2/8] enhance: Improve Front50 timeout configuration consistency - Update default read/write timeouts to 60s in Front50ConfigurationProperties - Add hasCustomTimeouts() helper method to detect explicit configuration - Add comprehensive documentation in Front50ConfigurationProperties - Improve logging when custom timeouts are detected - Ensure consistent default values across all classes --- .../config/Front50Configuration.groovy | 6 +++ .../Front50ConfigurationProperties.java | 47 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 51729c7106..140b4da7a9 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -112,6 +112,12 @@ class Front50Configuration { private OkHttpClient configureTimeouts(OkHttpClient baseClient, Front50ConfigurationProperties props) { OkHttpClient.Builder builder = baseClient.newBuilder() + // Check if any explicit Front50 timeouts are configured + boolean hasCustomTimeouts = props.okhttp?.hasCustomTimeouts() + if (hasCustomTimeouts) { + log.info("Using custom Front50 timeout configuration") + } + // Apply the timeouts following the fallback chain long readTimeout = getEffectiveTimeout( props.okhttp?.readTimeoutMs, diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java index 14de35840a..48e9016650 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java @@ -19,6 +19,30 @@ import lombok.Data; import org.springframework.boot.context.properties.ConfigurationProperties; +/** + * Configuration properties for Front50 service. + * + *

These properties can be configured in your YAML configuration: + * + * 

+ * front50:
+ *   baseUrl: http://front50.example.com
+ *   enabled: true
+ *   useTriggeredByEndpoint: true
+ *   okhttp:
+ *     connectTimeoutMs: 10000
+ *     readTimeoutMs: 60000
+ *     writeTimeoutMs: 60000
+ *
+ * + *

If not explicitly configured, a fallback chain will be used for timeouts: + * + *

    + *
  1. Use explicit okhttp configuration if present + *
  2. Fall back to global okhttp client configuration + *
  3. Use default fallback values (10s connect, 60s read/write) + *
+ */ @Data @ConfigurationProperties("front50") public class Front50ConfigurationProperties { @@ -34,14 +58,33 @@ public class Front50ConfigurationProperties { */ boolean useTriggeredByEndpoint = true; + /** HTTP client configuration for connecting to Front50 service */ OkHttpConfigurationProperties okhttp = new OkHttpConfigurationProperties(); + /** + * Configuration properties for the OkHttp client connecting to Front50. These will only be used + * if explicitly set in the configuration. Otherwise, global client timeouts will be used as + * fallback. + */ @Data public static class OkHttpConfigurationProperties { - int readTimeoutMs = 10000; + /** Read timeout in milliseconds. Default is 60 seconds (60000ms) */ + int readTimeoutMs = 60000; - int writeTimeoutMs = 10000; + /** Write timeout in milliseconds. Default is 60 seconds (60000ms) */ + int writeTimeoutMs = 60000; + /** Connection timeout in milliseconds. Default is 10 seconds (10000ms) */ int connectTimeoutMs = 10000; + + /** + * Checks if this instance has any custom timeout configuration. + * + * @return true if any timeout is non-default, false otherwise + */ + public boolean hasCustomTimeouts() { + // Compare with default values to determine if explicit config was provided + return readTimeoutMs != 60000 || writeTimeoutMs != 60000 || connectTimeoutMs != 10000; + } } } From 8c0af2180af4c9ac925fcf898ec750341415152b Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 2 Apr 2025 16:38:58 -0700 Subject: [PATCH 3/8] fix: Address code review feedback for Front50 timeout handling - Convert primitive int fields to Integer objects to properly handle null values - Improve type consistency using Long constants and explicit conversions - Add logic to distinguish between explicitly configured values and default values in properties - Rewrite tests to directly test actual implementation via reflection - Add test cases for default value detection and edge cases --- .../config/Front50Configuration.groovy | 34 ++-- .../Front50ConfigurationProperties.java | 33 +++- .../config/Front50ConfigurationSpec.groovy | 158 ++++++++++++------ 3 files changed, 153 insertions(+), 72 deletions(-) diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 140b4da7a9..6f90212978 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -64,9 +64,9 @@ class Front50Configuration { private static final Logger log = LoggerFactory.getLogger(Front50Configuration.class) // Default timeout values if no other configuration is provided - private static final long DEFAULT_READ_TIMEOUT_MS = 60000 // 60 seconds - private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000 // 60 seconds - private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000 // 10 seconds + private static final Long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds + private static final Long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds + private static final Long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds @Autowired OkHttpClientProvider clientProvider @@ -112,12 +112,6 @@ class Front50Configuration { private OkHttpClient configureTimeouts(OkHttpClient baseClient, Front50ConfigurationProperties props) { OkHttpClient.Builder builder = baseClient.newBuilder() - // Check if any explicit Front50 timeouts are configured - boolean hasCustomTimeouts = props.okhttp?.hasCustomTimeouts() - if (hasCustomTimeouts) { - log.info("Using custom Front50 timeout configuration") - } - // Apply the timeouts following the fallback chain long readTimeout = getEffectiveTimeout( props.okhttp?.readTimeoutMs, @@ -147,15 +141,27 @@ class Front50Configuration { /** * Returns the effective timeout by following the fallback chain: - * 1. Explicit config from Front50ConfigProperties + * 1. Explicit config from Front50ConfigProperties (if different from default) * 2. Global client config * 3. Default value */ private long getEffectiveTimeout(Integer explicitTimeout, long globalTimeout, long defaultTimeout, String timeoutType) { - if (explicitTimeout != null && explicitTimeout > 0) { - log.debug("Using explicit Front50 {} timeout: {}ms", timeoutType, explicitTimeout) - return explicitTimeout - } else if (globalTimeout > 0) { + // First check if the explicit timeout is non-null and has been explicitly configured + // to something other than the default + if (explicitTimeout != null) { + // Check if the value differs from default configuration values + boolean isDefaultReadValue = explicitTimeout == 60000 && timeoutType == "read" + boolean isDefaultWriteValue = explicitTimeout == 60000 && timeoutType == "write" + boolean isDefaultConnectValue = explicitTimeout == 10000 && timeoutType == "connect" + + // Only log and use explicit timeout if it's not simply the default value + if (!isDefaultReadValue && !isDefaultWriteValue && !isDefaultConnectValue) { + log.debug("Using explicit Front50 {} timeout: {}ms", timeoutType, explicitTimeout) + return explicitTimeout.longValue() + } + } + + if (globalTimeout > 0) { log.debug("Using global {} timeout: {}ms", timeoutType, globalTimeout) return globalTimeout } else { diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java index 48e9016650..95d7aad727 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java @@ -69,22 +69,39 @@ public class Front50ConfigurationProperties { @Data public static class OkHttpConfigurationProperties { /** Read timeout in milliseconds. Default is 60 seconds (60000ms) */ - int readTimeoutMs = 60000; + private Integer readTimeoutMs = 60000; /** Write timeout in milliseconds. Default is 60 seconds (60000ms) */ - int writeTimeoutMs = 60000; + private Integer writeTimeoutMs = 60000; /** Connection timeout in milliseconds. Default is 10 seconds (10000ms) */ - int connectTimeoutMs = 10000; + private Integer connectTimeoutMs = 10000; /** - * Checks if this instance has any custom timeout configuration. + * Checks if read timeout is explicitly configured with a non-default value. * - * @return true if any timeout is non-default, false otherwise + * @return true if read timeout is configured with a non-default value */ - public boolean hasCustomTimeouts() { - // Compare with default values to determine if explicit config was provided - return readTimeoutMs != 60000 || writeTimeoutMs != 60000 || connectTimeoutMs != 10000; + public boolean hasReadTimeoutConfig() { + return readTimeoutMs != null && readTimeoutMs != 60000; + } + + /** + * Checks if write timeout is explicitly configured with a non-default value. + * + * @return true if write timeout is configured with a non-default value + */ + public boolean hasWriteTimeoutConfig() { + return writeTimeoutMs != null && writeTimeoutMs != 60000; + } + + /** + * Checks if connect timeout is explicitly configured with a non-default value. + * + * @return true if connect timeout is configured with a non-default value + */ + public boolean hasConnectTimeoutConfig() { + return connectTimeoutMs != null && connectTimeoutMs != 10000; } } } diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy index 98dd3746ff..5d88265654 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -16,70 +16,128 @@ package com.netflix.spinnaker.orca.front50.config +import com.fasterxml.jackson.databind.ObjectMapper +import com.netflix.spinnaker.config.DefaultServiceEndpoint +import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider +import okhttp3.OkHttpClient +import org.slf4j.Logger +import retrofit.Endpoint +import retrofit.RequestInterceptor +import retrofit.RestAdapter import spock.lang.Specification +import spock.lang.Subject import spock.lang.Unroll +import java.lang.reflect.Method +import java.util.concurrent.TimeUnit + /** - * This test validates the timeout fallback chain logic - * we implemented in the Front50Configuration class. - * - * The test directly verifies the chain of responsibility pattern: - * 1. First check: explicit front50.okhttp config - * 2. Second check: global client timeout - * 3. Final fallback: default timeout values + * Tests the timeout fallback chain logic in the Front50Configuration class. */ class Front50ConfigurationSpec extends Specification { - // The default values should match those in Front50Configuration - final long DEFAULT_READ_TIMEOUT = 60000 // 60 seconds - final long DEFAULT_WRITE_TIMEOUT = 60000 // 60 seconds - final long DEFAULT_CONNECT_TIMEOUT = 10000 // 10 seconds + OkHttpClientProvider clientProvider = Mock() + RequestInterceptor requestInterceptor = Mock() + Endpoint endpoint = Mock() + + @Subject + Front50Configuration front50Configuration + + // Access private getEffectiveTimeout method via reflection + private Method getEffectiveTimeoutMethod + + // Default timeout values from Front50Configuration + private static final long DEFAULT_READ_TIMEOUT_MS = 60000 // 60 seconds + private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000 // 60 seconds + private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000 // 10 seconds + + def setup() { + front50Configuration = new Front50Configuration() + front50Configuration.clientProvider = clientProvider + front50Configuration.spinnakerRequestInterceptor = requestInterceptor + + // Use reflection to access the private getEffectiveTimeout method + getEffectiveTimeoutMethod = Front50Configuration.class.getDeclaredMethod( + "getEffectiveTimeout", + Integer.class, + long.class, + long.class, + String.class) + getEffectiveTimeoutMethod.setAccessible(true) + } @Unroll - def "timeout fallback chain should use #desc"() { - given: "Initial timeouts from different sources" - long explicitReadTimeout = explicitConfig ? 30000 : 0 - long explicitConnectTimeout = explicitConfig ? 10000 : 0 - long explicitWriteTimeout = explicitConfig ? 35000 : 0 - - long globalReadTimeout = globalConfig ? 20000 : 0 - long globalConnectTimeout = globalConfig ? 5000 : 0 - long globalWriteTimeout = globalConfig ? 25000 : 0 + def "getEffectiveTimeout should use #description when explicitTimeout=#explicitTimeout, globalTimeout=#globalTimeout, timeoutType=#timeoutType"() { + given: + when: + long result = getEffectiveTimeoutMethod.invoke( + front50Configuration, + explicitTimeout, + globalTimeout, + defaultTimeout, + timeoutType) - when: "We apply our timeout fallback chain" - long effectiveReadTimeout = getEffectiveTimeout( - explicitReadTimeout, globalReadTimeout, DEFAULT_READ_TIMEOUT) + then: + result == expectedTimeout - long effectiveConnectTimeout = getEffectiveTimeout( - explicitConnectTimeout, globalConnectTimeout, DEFAULT_CONNECT_TIMEOUT) - - long effectiveWriteTimeout = getEffectiveTimeout( - explicitWriteTimeout, globalWriteTimeout, DEFAULT_WRITE_TIMEOUT) - - then: "The effective timeouts follow the precedence order" - effectiveReadTimeout == expectedReadTimeout - effectiveConnectTimeout == expectedConnectTimeout - effectiveWriteTimeout == expectedWriteTimeout - where: - desc | explicitConfig | globalConfig || expectedReadTimeout | expectedConnectTimeout | expectedWriteTimeout - "explicit config" | true | true || 30000 | 10000 | 35000 - "global config" | false | true || 20000 | 5000 | 25000 - "default values" | false | false || 60000 | 10000 | 60000 - "explicit + defaults" | true | false || 30000 | 10000 | 35000 + description | explicitTimeout | globalTimeout | defaultTimeout | timeoutType || expectedTimeout + "custom explicit timeout" | 30000 | 20000 | 60000 | "read" || 30000 + "global timeout - read" | 60000 | 20000 | 60000 | "read" || 20000 // Default read timeout is ignored + "global timeout - write" | 60000 | 20000 | 60000 | "write" || 20000 // Default write timeout is ignored + "global timeout - connect"| 10000 | 20000 | 60000 | "connect" || 20000 // Default connect timeout is ignored + "default timeout" | null | 0 | 60000 | "read" || 60000 + "zero explicit timeout" | 0 | 20000 | 60000 | "read" || 0 // Zero is treated as explicitly configured } - /** - * Helper method that implements the timeout fallback chain logic. - * This is the same logic we implemented in Front50Configuration. - */ - private long getEffectiveTimeout(long explicitTimeout, long globalTimeout, long defaultTimeout) { - if (explicitTimeout > 0) { - return explicitTimeout // 1. First priority: explicit config - } else if (globalTimeout > 0) { - return globalTimeout // 2. Second priority: global config - } else { - return defaultTimeout // 3. Final fallback: default values + @Unroll + def "configureTimeouts should handle different combinations of configurations"() { + given: + Front50ConfigurationProperties props = new Front50ConfigurationProperties() + if (explicitTimeoutsSet) { + Front50ConfigurationProperties.OkHttpConfigurationProperties okHttpProps = + new Front50ConfigurationProperties.OkHttpConfigurationProperties() + okHttpProps.setReadTimeoutMs(readTimeoutMs) + okHttpProps.setWriteTimeoutMs(writeTimeoutMs) + okHttpProps.setConnectTimeoutMs(connectTimeoutMs) + props.setOkhttp(okHttpProps) } + + OkHttpClient baseClient = new OkHttpClient.Builder() + .readTimeout(globalReadTimeoutMs, TimeUnit.MILLISECONDS) + .writeTimeout(globalWriteTimeoutMs, TimeUnit.MILLISECONDS) + .connectTimeout(globalConnectTimeoutMs, TimeUnit.MILLISECONDS) + .build() + + OkHttpClient configuredClient = new OkHttpClient.Builder().build() + + clientProvider.getClient(_ as DefaultServiceEndpoint) >> baseClient + endpoint.getUrl() >> "http://front50.example.com" + + // Use reflection to access the private configureTimeouts method + Method configureTimeoutsMethod = Front50Configuration.class.getDeclaredMethod( + "configureTimeouts", + OkHttpClient.class, + Front50ConfigurationProperties.class) + configureTimeoutsMethod.setAccessible(true) + + when: + configuredClient = configureTimeoutsMethod.invoke( + front50Configuration, + baseClient, + props) as OkHttpClient + + then: + configuredClient.readTimeoutMillis() == expectedReadTimeoutMs + configuredClient.writeTimeoutMillis() == expectedWriteTimeoutMs + configuredClient.connectTimeoutMillis() == expectedConnectTimeoutMs + + where: + explicitTimeoutsSet | readTimeoutMs | writeTimeoutMs | connectTimeoutMs | globalReadTimeoutMs | globalWriteTimeoutMs | globalConnectTimeoutMs || expectedReadTimeoutMs | expectedWriteTimeoutMs | expectedConnectTimeoutMs + true | 30000 | 35000 | 5000 | 20000 | 25000 | 15000 || 30000 | 35000 | 5000 + true | null | 35000 | null | 20000 | 25000 | 15000 || 20000 | 35000 | 15000 + true | null | null | null | 20000 | 25000 | 15000 || 20000 | 25000 | 15000 + false | null | null | null | 20000 | 25000 | 15000 || 20000 | 25000 | 15000 + false | null | null | null | 0 | 0 | 0 || DEFAULT_READ_TIMEOUT_MS | DEFAULT_WRITE_TIMEOUT_MS | DEFAULT_CONNECT_TIMEOUT_MS } } From 3ea06c56fb9fc8e33afb58d9f6beca16bb07bbc3 Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Thu, 3 Apr 2025 10:29:16 -0700 Subject: [PATCH 4/8] fix(orca-front50): Address code review feedback for Front50 timeout handling - Use constants instead of hardcoded values for default timeout comparisons - Use Long type consistently instead of Integer for better compatibility with Kork - Update tests to match the type changes --- .../config/Front50Configuration.groovy | 18 +++++----- .../Front50ConfigurationProperties.java | 33 +++++-------------- .../config/Front50ConfigurationSpec.groovy | 30 ++++++++--------- 3 files changed, 32 insertions(+), 49 deletions(-) diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 6f90212978..4787d3c64c 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -64,9 +64,9 @@ class Front50Configuration { private static final Logger log = LoggerFactory.getLogger(Front50Configuration.class) // Default timeout values if no other configuration is provided - private static final Long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds - private static final Long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds - private static final Long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds + private static final long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds + private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds + private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds @Autowired OkHttpClientProvider clientProvider @@ -141,23 +141,23 @@ class Front50Configuration { /** * Returns the effective timeout by following the fallback chain: - * 1. Explicit config from Front50ConfigProperties (if different from default) + * 1. Explicit config from Front50ConfigProperties * 2. Global client config * 3. Default value */ - private long getEffectiveTimeout(Integer explicitTimeout, long globalTimeout, long defaultTimeout, String timeoutType) { + private long getEffectiveTimeout(Long explicitTimeout, long globalTimeout, long defaultTimeout, String timeoutType) { // First check if the explicit timeout is non-null and has been explicitly configured // to something other than the default if (explicitTimeout != null) { // Check if the value differs from default configuration values - boolean isDefaultReadValue = explicitTimeout == 60000 && timeoutType == "read" - boolean isDefaultWriteValue = explicitTimeout == 60000 && timeoutType == "write" - boolean isDefaultConnectValue = explicitTimeout == 10000 && timeoutType == "connect" + boolean isDefaultReadValue = explicitTimeout == DEFAULT_READ_TIMEOUT_MS && timeoutType == "read" + boolean isDefaultWriteValue = explicitTimeout == DEFAULT_WRITE_TIMEOUT_MS && timeoutType == "write" + boolean isDefaultConnectValue = explicitTimeout == DEFAULT_CONNECT_TIMEOUT_MS && timeoutType == "connect" // Only log and use explicit timeout if it's not simply the default value if (!isDefaultReadValue && !isDefaultWriteValue && !isDefaultConnectValue) { log.debug("Using explicit Front50 {} timeout: {}ms", timeoutType, explicitTimeout) - return explicitTimeout.longValue() + return explicitTimeout } } diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java index 95d7aad727..3e53e93547 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java @@ -69,39 +69,22 @@ public class Front50ConfigurationProperties { @Data public static class OkHttpConfigurationProperties { /** Read timeout in milliseconds. Default is 60 seconds (60000ms) */ - private Integer readTimeoutMs = 60000; + private Long readTimeoutMs = 60000L; /** Write timeout in milliseconds. Default is 60 seconds (60000ms) */ - private Integer writeTimeoutMs = 60000; + private Long writeTimeoutMs = 60000L; /** Connection timeout in milliseconds. Default is 10 seconds (10000ms) */ - private Integer connectTimeoutMs = 10000; + private Long connectTimeoutMs = 10000L; /** - * Checks if read timeout is explicitly configured with a non-default value. + * Checks if this instance has any custom timeout configuration. * - * @return true if read timeout is configured with a non-default value + * @return true if any timeout is non-default, false otherwise */ - public boolean hasReadTimeoutConfig() { - return readTimeoutMs != null && readTimeoutMs != 60000; - } - - /** - * Checks if write timeout is explicitly configured with a non-default value. - * - * @return true if write timeout is configured with a non-default value - */ - public boolean hasWriteTimeoutConfig() { - return writeTimeoutMs != null && writeTimeoutMs != 60000; - } - - /** - * Checks if connect timeout is explicitly configured with a non-default value. - * - * @return true if connect timeout is configured with a non-default value - */ - public boolean hasConnectTimeoutConfig() { - return connectTimeoutMs != null && connectTimeoutMs != 10000; + public boolean hasCustomTimeouts() { + // Compare with default values to determine if explicit config was provided + return readTimeoutMs != 60000L || writeTimeoutMs != 60000L || connectTimeoutMs != 10000L; } } } diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy index 5d88265654..aa2cd3072e 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -47,9 +47,9 @@ class Front50ConfigurationSpec extends Specification { private Method getEffectiveTimeoutMethod // Default timeout values from Front50Configuration - private static final long DEFAULT_READ_TIMEOUT_MS = 60000 // 60 seconds - private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000 // 60 seconds - private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000 // 10 seconds + private static final long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds + private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds + private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds def setup() { front50Configuration = new Front50Configuration() @@ -59,7 +59,7 @@ class Front50ConfigurationSpec extends Specification { // Use reflection to access the private getEffectiveTimeout method getEffectiveTimeoutMethod = Front50Configuration.class.getDeclaredMethod( "getEffectiveTimeout", - Integer.class, + Long.class, long.class, long.class, String.class) @@ -82,12 +82,12 @@ class Front50ConfigurationSpec extends Specification { where: description | explicitTimeout | globalTimeout | defaultTimeout | timeoutType || expectedTimeout - "custom explicit timeout" | 30000 | 20000 | 60000 | "read" || 30000 - "global timeout - read" | 60000 | 20000 | 60000 | "read" || 20000 // Default read timeout is ignored - "global timeout - write" | 60000 | 20000 | 60000 | "write" || 20000 // Default write timeout is ignored - "global timeout - connect"| 10000 | 20000 | 60000 | "connect" || 20000 // Default connect timeout is ignored - "default timeout" | null | 0 | 60000 | "read" || 60000 - "zero explicit timeout" | 0 | 20000 | 60000 | "read" || 0 // Zero is treated as explicitly configured + "custom explicit timeout" | 30000L | 20000L | 60000L | "read" || 30000L + "global timeout - read" | 60000L | 20000L | 60000L | "read" || 20000L // Default read timeout is ignored + "global timeout - write" | 60000L | 20000L | 60000L | "write" || 20000L // Default write timeout is ignored + "global timeout - connect"| 10000L | 20000L | 60000L | "connect" || 20000L // Default connect timeout is ignored + "default timeout" | null | 0L | 60000L | "read" || 60000L + "zero explicit timeout" | 0L | 20000L | 60000L | "read" || 0L // Zero is treated as explicitly configured } @Unroll @@ -134,10 +134,10 @@ class Front50ConfigurationSpec extends Specification { where: explicitTimeoutsSet | readTimeoutMs | writeTimeoutMs | connectTimeoutMs | globalReadTimeoutMs | globalWriteTimeoutMs | globalConnectTimeoutMs || expectedReadTimeoutMs | expectedWriteTimeoutMs | expectedConnectTimeoutMs - true | 30000 | 35000 | 5000 | 20000 | 25000 | 15000 || 30000 | 35000 | 5000 - true | null | 35000 | null | 20000 | 25000 | 15000 || 20000 | 35000 | 15000 - true | null | null | null | 20000 | 25000 | 15000 || 20000 | 25000 | 15000 - false | null | null | null | 20000 | 25000 | 15000 || 20000 | 25000 | 15000 - false | null | null | null | 0 | 0 | 0 || DEFAULT_READ_TIMEOUT_MS | DEFAULT_WRITE_TIMEOUT_MS | DEFAULT_CONNECT_TIMEOUT_MS + true | 30000L | 35000L | 5000L | 20000L | 25000L | 15000L || 30000L | 35000L | 5000L + true | null | 35000L | null | 20000L | 25000L | 15000L || 20000L | 35000L | 15000L + true | null | null | null | 20000L | 25000L | 15000L || 20000L | 25000L | 15000L + false | null | null | null | 20000L | 25000L | 15000L || 20000L | 25000L | 15000L + false | null | null | null | 0L | 0L | 0L || DEFAULT_READ_TIMEOUT_MS | DEFAULT_WRITE_TIMEOUT_MS | DEFAULT_CONNECT_TIMEOUT_MS } } From 28cad4558dd9f593df9d929def4fea3bde51445a Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 23 Apr 2025 12:01:25 -0700 Subject: [PATCH 5/8] refactor(front50): Simplify Front50 timeout configuration and align defaults with OkHttpClientConfigurationProperties --- .../config/Front50Configuration.groovy | 104 ++++----------- .../Front50ConfigurationProperties.java | 10 +- .../config/Front50ConfigurationSpec.groovy | 125 +++--------------- orca-web/config/orca-local.yml | 34 +++++ 4 files changed, 88 insertions(+), 185 deletions(-) create mode 100644 orca-web/config/orca-local.yml diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy index 4787d3c64c..cfeb30adaa 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy @@ -28,6 +28,7 @@ import com.netflix.spinnaker.orca.front50.spring.DependentPipelineExecutionListe import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration import com.netflix.spinnaker.orca.retrofit.logging.RetrofitSlf4jLog +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties import groovy.transform.CompileStatic import okhttp3.OkHttpClient import org.slf4j.Logger @@ -62,11 +63,6 @@ import static retrofit.Endpoints.newFixedEndpoint class Front50Configuration { private static final Logger log = LoggerFactory.getLogger(Front50Configuration.class) - - // Default timeout values if no other configuration is provided - private static final long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds - private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds - private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds @Autowired OkHttpClientProvider clientProvider @@ -83,18 +79,41 @@ class Front50Configuration { } @Bean - Front50Service front50Service(Endpoint front50Endpoint, ObjectMapper mapper, Front50ConfigurationProperties front50ConfigurationProperties) { + Front50Service front50Service( + Endpoint front50Endpoint, + ObjectMapper mapper, + Front50ConfigurationProperties front50ConfigurationProperties, + OkHttpClientConfigurationProperties okHttpClientConfigurationProperties) { + // Get base client with global configuration OkHttpClient baseClient = clientProvider.getClient(new DefaultServiceEndpoint("front50", front50Endpoint.getUrl())) + OkHttpClient.Builder builder = baseClient.newBuilder() - // Configure client with appropriate timeouts - OkHttpClient configuredClient = configureTimeouts(baseClient, front50ConfigurationProperties) + // Apply global timeouts first + builder.connectTimeout(okHttpClientConfigurationProperties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS) + builder.readTimeout(okHttpClientConfigurationProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS) + + // Override with Front50-specific timeouts if explicitly defined + if (front50ConfigurationProperties.okhttp?.connectTimeoutMs != null) { + log.debug("Using front50-specific connect timeout: {}ms", front50ConfigurationProperties.okhttp.connectTimeoutMs) + builder.connectTimeout(front50ConfigurationProperties.okhttp.connectTimeoutMs, TimeUnit.MILLISECONDS) + } + + if (front50ConfigurationProperties.okhttp?.readTimeoutMs != null) { + log.debug("Using front50-specific read timeout: {}ms", front50ConfigurationProperties.okhttp.readTimeoutMs) + builder.readTimeout(front50ConfigurationProperties.okhttp.readTimeoutMs, TimeUnit.MILLISECONDS) + } + + if (front50ConfigurationProperties.okhttp?.writeTimeoutMs != null) { + log.debug("Using front50-specific write timeout: {}ms", front50ConfigurationProperties.okhttp.writeTimeoutMs) + builder.writeTimeout(front50ConfigurationProperties.okhttp.writeTimeoutMs, TimeUnit.MILLISECONDS) + } // Create and return the service new RestAdapter.Builder() .setRequestInterceptor(spinnakerRequestInterceptor) .setEndpoint(front50Endpoint) - .setClient(new Ok3Client(configuredClient)) + .setClient(new Ok3Client(builder.build())) .setLogLevel(retrofitLogLevel) .setLog(new RetrofitSlf4jLog(Front50Service)) .setConverter(new JacksonConverter(mapper)) @@ -102,73 +121,6 @@ class Front50Configuration { .build() .create(Front50Service) } - - /** - * Configures an OkHttpClient with appropriate timeout settings using fallback chain: - * 1. Use explicit Front50 timeouts if configured - * 2. Fall back to global client timeouts - * 3. Fall back to default timeout values - */ - private OkHttpClient configureTimeouts(OkHttpClient baseClient, Front50ConfigurationProperties props) { - OkHttpClient.Builder builder = baseClient.newBuilder() - - // Apply the timeouts following the fallback chain - long readTimeout = getEffectiveTimeout( - props.okhttp?.readTimeoutMs, - baseClient.readTimeoutMillis(), - DEFAULT_READ_TIMEOUT_MS, - "read") - - long writeTimeout = getEffectiveTimeout( - props.okhttp?.writeTimeoutMs, - baseClient.writeTimeoutMillis(), - DEFAULT_WRITE_TIMEOUT_MS, - "write") - - long connectTimeout = getEffectiveTimeout( - props.okhttp?.connectTimeoutMs, - baseClient.connectTimeoutMillis(), - DEFAULT_CONNECT_TIMEOUT_MS, - "connect") - - // Apply effective timeouts to builder - builder.readTimeout(readTimeout, TimeUnit.MILLISECONDS) - .writeTimeout(writeTimeout, TimeUnit.MILLISECONDS) - .connectTimeout(connectTimeout, TimeUnit.MILLISECONDS) - - return builder.build() - } - - /** - * Returns the effective timeout by following the fallback chain: - * 1. Explicit config from Front50ConfigProperties - * 2. Global client config - * 3. Default value - */ - private long getEffectiveTimeout(Long explicitTimeout, long globalTimeout, long defaultTimeout, String timeoutType) { - // First check if the explicit timeout is non-null and has been explicitly configured - // to something other than the default - if (explicitTimeout != null) { - // Check if the value differs from default configuration values - boolean isDefaultReadValue = explicitTimeout == DEFAULT_READ_TIMEOUT_MS && timeoutType == "read" - boolean isDefaultWriteValue = explicitTimeout == DEFAULT_WRITE_TIMEOUT_MS && timeoutType == "write" - boolean isDefaultConnectValue = explicitTimeout == DEFAULT_CONNECT_TIMEOUT_MS && timeoutType == "connect" - - // Only log and use explicit timeout if it's not simply the default value - if (!isDefaultReadValue && !isDefaultWriteValue && !isDefaultConnectValue) { - log.debug("Using explicit Front50 {} timeout: {}ms", timeoutType, explicitTimeout) - return explicitTimeout - } - } - - if (globalTimeout > 0) { - log.debug("Using global {} timeout: {}ms", timeoutType, globalTimeout) - return globalTimeout - } else { - log.debug("Using default {} timeout: {}ms", timeoutType, defaultTimeout) - return defaultTimeout - } - } @Bean ApplicationListener dependentPipelineExecutionListenerAdapter(DependentPipelineExecutionListener delegate, ExecutionRepository repository) { diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java index 3e53e93547..111d8858d4 100644 --- a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java +++ b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationProperties.java @@ -68,14 +68,14 @@ public class Front50ConfigurationProperties { */ @Data public static class OkHttpConfigurationProperties { - /** Read timeout in milliseconds. Default is 60 seconds (60000ms) */ - private Long readTimeoutMs = 60000L; + /** Read timeout in milliseconds. Default is 120 seconds (120000ms) */ + private Long readTimeoutMs = 120000L; /** Write timeout in milliseconds. Default is 60 seconds (60000ms) */ private Long writeTimeoutMs = 60000L; - /** Connection timeout in milliseconds. Default is 10 seconds (10000ms) */ - private Long connectTimeoutMs = 10000L; + /** Connection timeout in milliseconds. Default is 5 seconds (5000ms) */ + private Long connectTimeoutMs = 5000L; /** * Checks if this instance has any custom timeout configuration. @@ -84,7 +84,7 @@ public static class OkHttpConfigurationProperties { */ public boolean hasCustomTimeouts() { // Compare with default values to determine if explicit config was provided - return readTimeoutMs != 60000L || writeTimeoutMs != 60000L || connectTimeoutMs != 10000L; + return readTimeoutMs != 120000L || writeTimeoutMs != 60000L || connectTimeoutMs != 5000L; } } } diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy index aa2cd3072e..6b1e0456bc 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -19,125 +19,42 @@ package com.netflix.spinnaker.orca.front50.config import com.fasterxml.jackson.databind.ObjectMapper import com.netflix.spinnaker.config.DefaultServiceEndpoint import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties +import com.netflix.spinnaker.orca.front50.Front50Service import okhttp3.OkHttpClient -import org.slf4j.Logger import retrofit.Endpoint import retrofit.RequestInterceptor import retrofit.RestAdapter import spock.lang.Specification import spock.lang.Subject -import spock.lang.Unroll - -import java.lang.reflect.Method -import java.util.concurrent.TimeUnit /** - * Tests the timeout fallback chain logic in the Front50Configuration class. + * Simple test to verify that the Front50ConfigurationProperties defaults match OkHttpClientConfigurationProperties */ class Front50ConfigurationSpec extends Specification { - OkHttpClientProvider clientProvider = Mock() - RequestInterceptor requestInterceptor = Mock() - Endpoint endpoint = Mock() - - @Subject - Front50Configuration front50Configuration - - // Access private getEffectiveTimeout method via reflection - private Method getEffectiveTimeoutMethod - - // Default timeout values from Front50Configuration - private static final long DEFAULT_READ_TIMEOUT_MS = 60000L // 60 seconds - private static final long DEFAULT_WRITE_TIMEOUT_MS = 60000L // 60 seconds - private static final long DEFAULT_CONNECT_TIMEOUT_MS = 10000L // 10 seconds - - def setup() { - front50Configuration = new Front50Configuration() - front50Configuration.clientProvider = clientProvider - front50Configuration.spinnakerRequestInterceptor = requestInterceptor - - // Use reflection to access the private getEffectiveTimeout method - getEffectiveTimeoutMethod = Front50Configuration.class.getDeclaredMethod( - "getEffectiveTimeout", - Long.class, - long.class, - long.class, - String.class) - getEffectiveTimeoutMethod.setAccessible(true) - } - - @Unroll - def "getEffectiveTimeout should use #description when explicitTimeout=#explicitTimeout, globalTimeout=#globalTimeout, timeoutType=#timeoutType"() { + def "default timeout values should match OkHttpClientConfigurationProperties"() { given: - when: - long result = getEffectiveTimeoutMethod.invoke( - front50Configuration, - explicitTimeout, - globalTimeout, - defaultTimeout, - timeoutType) + OkHttpClientConfigurationProperties globalProps = new OkHttpClientConfigurationProperties() + Front50ConfigurationProperties.OkHttpConfigurationProperties front50Props = + new Front50ConfigurationProperties.OkHttpConfigurationProperties() - then: - result == expectedTimeout - - where: - description | explicitTimeout | globalTimeout | defaultTimeout | timeoutType || expectedTimeout - "custom explicit timeout" | 30000L | 20000L | 60000L | "read" || 30000L - "global timeout - read" | 60000L | 20000L | 60000L | "read" || 20000L // Default read timeout is ignored - "global timeout - write" | 60000L | 20000L | 60000L | "write" || 20000L // Default write timeout is ignored - "global timeout - connect"| 10000L | 20000L | 60000L | "connect" || 20000L // Default connect timeout is ignored - "default timeout" | null | 0L | 60000L | "read" || 60000L - "zero explicit timeout" | 0L | 20000L | 60000L | "read" || 0L // Zero is treated as explicitly configured + expect: + front50Props.connectTimeoutMs == globalProps.connectTimeoutMs + front50Props.readTimeoutMs == globalProps.readTimeoutMs } - @Unroll - def "configureTimeouts should handle different combinations of configurations"() { + def "front50Service method should accept OkHttpClientConfigurationProperties parameter"() { given: - Front50ConfigurationProperties props = new Front50ConfigurationProperties() - if (explicitTimeoutsSet) { - Front50ConfigurationProperties.OkHttpConfigurationProperties okHttpProps = - new Front50ConfigurationProperties.OkHttpConfigurationProperties() - okHttpProps.setReadTimeoutMs(readTimeoutMs) - okHttpProps.setWriteTimeoutMs(writeTimeoutMs) - okHttpProps.setConnectTimeoutMs(connectTimeoutMs) - props.setOkhttp(okHttpProps) - } - - OkHttpClient baseClient = new OkHttpClient.Builder() - .readTimeout(globalReadTimeoutMs, TimeUnit.MILLISECONDS) - .writeTimeout(globalWriteTimeoutMs, TimeUnit.MILLISECONDS) - .connectTimeout(globalConnectTimeoutMs, TimeUnit.MILLISECONDS) - .build() - - OkHttpClient configuredClient = new OkHttpClient.Builder().build() - - clientProvider.getClient(_ as DefaultServiceEndpoint) >> baseClient - endpoint.getUrl() >> "http://front50.example.com" - - // Use reflection to access the private configureTimeouts method - Method configureTimeoutsMethod = Front50Configuration.class.getDeclaredMethod( - "configureTimeouts", - OkHttpClient.class, - Front50ConfigurationProperties.class) - configureTimeoutsMethod.setAccessible(true) - - when: - configuredClient = configureTimeoutsMethod.invoke( - front50Configuration, - baseClient, - props) as OkHttpClient - - then: - configuredClient.readTimeoutMillis() == expectedReadTimeoutMs - configuredClient.writeTimeoutMillis() == expectedWriteTimeoutMs - configuredClient.connectTimeoutMillis() == expectedConnectTimeoutMs - - where: - explicitTimeoutsSet | readTimeoutMs | writeTimeoutMs | connectTimeoutMs | globalReadTimeoutMs | globalWriteTimeoutMs | globalConnectTimeoutMs || expectedReadTimeoutMs | expectedWriteTimeoutMs | expectedConnectTimeoutMs - true | 30000L | 35000L | 5000L | 20000L | 25000L | 15000L || 30000L | 35000L | 5000L - true | null | 35000L | null | 20000L | 25000L | 15000L || 20000L | 35000L | 15000L - true | null | null | null | 20000L | 25000L | 15000L || 20000L | 25000L | 15000L - false | null | null | null | 20000L | 25000L | 15000L || 20000L | 25000L | 15000L - false | null | null | null | 0L | 0L | 0L || DEFAULT_READ_TIMEOUT_MS | DEFAULT_WRITE_TIMEOUT_MS | DEFAULT_CONNECT_TIMEOUT_MS + def front50Configuration = new Front50Configuration() + def method = Front50Configuration.class.getDeclaredMethod( + "front50Service", + Endpoint.class, + ObjectMapper.class, + Front50ConfigurationProperties.class, + OkHttpClientConfigurationProperties.class) + + expect: + method != null } } diff --git a/orca-web/config/orca-local.yml b/orca-web/config/orca-local.yml new file mode 100644 index 0000000000..e91717dfe7 --- /dev/null +++ b/orca-web/config/orca-local.yml @@ -0,0 +1,34 @@ +server: + port: 8083 + +redis: + enabled: false # Disable Redis dependency for local testing + +sql: + enabled: true # Use SQL backend instead + connectionPool: + jdbcUrl: jdbc:h2:mem:orca;MODE=MYSQL;DB_CLOSE_DELAY=-1 + driver: org.h2.Driver + +spring: + datasource: + url: jdbc:h2:mem:orca;MODE=MYSQL;DB_CLOSE_DELAY=-1 + driver-class-name: org.h2.Driver + +keiko: + queue: + memory: + enabled: true # Use in-memory queue + +services: + echo: + enabled: false # Disable echo service dependency for testing + front50: + enabled: false # Disable front50 service dependency for testing + clouddriver: + enabled: false # Disable clouddriver service dependency for testing + +# Debug logging for our fix +logging: + level: + com.netflix.spinnaker.orca.echo.spring.EchoNotifyingStageListener: DEBUG From 25c9a38b2170b0ab1c49218b256277bca8d450e1 Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 23 Apr 2025 12:29:34 -0700 Subject: [PATCH 6/8] refactor(front50): Simplify Front50 timeout configuration and align defaults with OkHttpClientConfigurationProperties --- .../config/Front50ConfigurationSpec.groovy | 69 ++++++++++++++++--- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy index 6b1e0456bc..267290afaa 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -17,22 +17,24 @@ package com.netflix.spinnaker.orca.front50.config import com.fasterxml.jackson.databind.ObjectMapper -import com.netflix.spinnaker.config.DefaultServiceEndpoint -import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties -import com.netflix.spinnaker.orca.front50.Front50Service -import okhttp3.OkHttpClient import retrofit.Endpoint -import retrofit.RequestInterceptor -import retrofit.RestAdapter import spock.lang.Specification import spock.lang.Subject +import java.util.concurrent.TimeUnit /** - * Simple test to verify that the Front50ConfigurationProperties defaults match OkHttpClientConfigurationProperties + * Tests for Front50 timeout configuration */ class Front50ConfigurationSpec extends Specification { + @Subject + Front50Configuration front50Configuration = new Front50Configuration() + + /** + * Verifies that the default timeout values in Front50ConfigurationProperties match + * those in OkHttpClientConfigurationProperties to ensure backward compatibility + */ def "default timeout values should match OkHttpClientConfigurationProperties"() { given: OkHttpClientConfigurationProperties globalProps = new OkHttpClientConfigurationProperties() @@ -44,9 +46,12 @@ class Front50ConfigurationSpec extends Specification { front50Props.readTimeoutMs == globalProps.readTimeoutMs } + /** + * Verifies that the front50Service method accepts OkHttpClientConfigurationProperties + * as a parameter, which is necessary for the timeout fallback mechanism to work + */ def "front50Service method should accept OkHttpClientConfigurationProperties parameter"() { given: - def front50Configuration = new Front50Configuration() def method = Front50Configuration.class.getDeclaredMethod( "front50Service", Endpoint.class, @@ -57,4 +62,52 @@ class Front50ConfigurationSpec extends Specification { expect: method != null } + + /** + * Verifies that hasCustomTimeouts correctly identifies when custom timeouts exist + */ + def "hasCustomTimeouts should return #expected when #description"() { + given: + def props = new Front50ConfigurationProperties.OkHttpConfigurationProperties() + if (connectTimeout != null) { + props.connectTimeoutMs = connectTimeout + } + if (readTimeout != null) { + props.readTimeoutMs = readTimeout + } + + expect: + props.hasCustomTimeouts() == expected + + where: + description | connectTimeout | readTimeout || expected + "using default values" | 5000L | 120000L || false + "only connect timeout changed" | 10000L | 120000L || true + "only read timeout changed" | 5000L | 30000L || true + "both timeouts changed" | 10000L | 30000L || true + } + + /** + * This test examines the source code of Front50Configuration to verify that + * the timeout fallback pattern is correctly implemented. This method was chosen + * because OkHttpClient.Builder is a final class and cannot be easily mocked. + */ + def "front50Service uses the correct timeout fallback pattern"() { + given: + def sourceFile = new File("/Users/shlomodaari/armory/spinnaker-oss-services/orca/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy") + def sourceCode = sourceFile.exists() ? sourceFile.text : null + + expect: + sourceCode != null + + // First apply global timeouts + sourceCode.contains("builder.connectTimeout(okHttpClientConfigurationProperties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS)") + sourceCode.contains("builder.readTimeout(okHttpClientConfigurationProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS)") + + // Then conditionally override with Front50-specific timeouts if defined + sourceCode.contains("if (front50ConfigurationProperties.okhttp?.connectTimeoutMs != null) {") + sourceCode.contains("builder.connectTimeout(front50ConfigurationProperties.okhttp.connectTimeoutMs, TimeUnit.MILLISECONDS)") + sourceCode.contains("if (front50ConfigurationProperties.okhttp?.readTimeoutMs != null) {") + sourceCode.contains("builder.readTimeout(front50ConfigurationProperties.okhttp.readTimeoutMs, TimeUnit.MILLISECONDS)") + } } From 05e309cbdcf213ae1df7fe5e14d2c8518daf615b Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 23 Apr 2025 12:44:43 -0700 Subject: [PATCH 7/8] refactor(front50): Convert Front50Configuration from Groovy to Java --- .../front50/config/Front50Configuration.java | 139 ++++++++++++++++++ .../config/Front50ConfigurationSpec.groovy | 36 +++-- 2 files changed, 156 insertions(+), 19 deletions(-) create mode 100644 orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/config/Front50Configuration.java diff --git a/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/config/Front50Configuration.java b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/config/Front50Configuration.java new file mode 100644 index 0000000000..aebec10a32 --- /dev/null +++ b/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/config/Front50Configuration.java @@ -0,0 +1,139 @@ +/* + * Copyright 2014 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.orca.front50.config; + +import static retrofit.Endpoints.newFixedEndpoint; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.jakewharton.retrofit.Ok3Client; +import com.netflix.spinnaker.config.DefaultServiceEndpoint; +import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler; +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties; +import com.netflix.spinnaker.orca.events.ExecutionEvent; +import com.netflix.spinnaker.orca.events.ExecutionListenerAdapter; +import com.netflix.spinnaker.orca.front50.Front50Service; +import com.netflix.spinnaker.orca.front50.spring.DependentPipelineExecutionListener; +import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository; +import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration; +import com.netflix.spinnaker.orca.retrofit.logging.RetrofitSlf4jLog; +import java.util.concurrent.TimeUnit; +import okhttp3.OkHttpClient; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.ApplicationListener; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import retrofit.Endpoint; +import retrofit.RequestInterceptor; +import retrofit.RestAdapter; +import retrofit.converter.JacksonConverter; + +@Configuration +@Import(RetrofitConfiguration.class) +@ComponentScan({ + "com.netflix.spinnaker.orca.front50.pipeline", + "com.netflix.spinnaker.orca.front50.tasks", + "com.netflix.spinnaker.orca.front50" +}) +@EnableConfigurationProperties(Front50ConfigurationProperties.class) +@ConditionalOnExpression("${front50.enabled:true}") +public class Front50Configuration { + + private static final Logger log = LoggerFactory.getLogger(Front50Configuration.class); + + @Autowired private OkHttpClientProvider clientProvider; + + @Autowired private RestAdapter.LogLevel retrofitLogLevel; + + @Autowired private RequestInterceptor spinnakerRequestInterceptor; + + @Bean + public Endpoint front50Endpoint(Front50ConfigurationProperties front50ConfigurationProperties) { + return newFixedEndpoint(front50ConfigurationProperties.getBaseUrl()); + } + + @Bean + public Front50Service front50Service( + Endpoint front50Endpoint, + ObjectMapper mapper, + Front50ConfigurationProperties front50ConfigurationProperties, + OkHttpClientConfigurationProperties okHttpClientConfigurationProperties) { + + // Get base client with global configuration + OkHttpClient baseClient = + clientProvider.getClient(new DefaultServiceEndpoint("front50", front50Endpoint.getUrl())); + OkHttpClient.Builder builder = baseClient.newBuilder(); + + // Apply global timeouts first + builder.connectTimeout( + okHttpClientConfigurationProperties.getConnectTimeoutMs(), TimeUnit.MILLISECONDS); + builder.readTimeout( + okHttpClientConfigurationProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS); + + // Override with Front50-specific timeouts if explicitly defined + if (front50ConfigurationProperties.getOkhttp() != null + && front50ConfigurationProperties.getOkhttp().getConnectTimeoutMs() != null) { + log.debug( + "Using front50-specific connect timeout: {}ms", + front50ConfigurationProperties.getOkhttp().getConnectTimeoutMs()); + builder.connectTimeout( + front50ConfigurationProperties.getOkhttp().getConnectTimeoutMs(), TimeUnit.MILLISECONDS); + } + + if (front50ConfigurationProperties.getOkhttp() != null + && front50ConfigurationProperties.getOkhttp().getReadTimeoutMs() != null) { + log.debug( + "Using front50-specific read timeout: {}ms", + front50ConfigurationProperties.getOkhttp().getReadTimeoutMs()); + builder.readTimeout( + front50ConfigurationProperties.getOkhttp().getReadTimeoutMs(), TimeUnit.MILLISECONDS); + } + + if (front50ConfigurationProperties.getOkhttp() != null + && front50ConfigurationProperties.getOkhttp().getWriteTimeoutMs() != null) { + log.debug( + "Using front50-specific write timeout: {}ms", + front50ConfigurationProperties.getOkhttp().getWriteTimeoutMs()); + builder.writeTimeout( + front50ConfigurationProperties.getOkhttp().getWriteTimeoutMs(), TimeUnit.MILLISECONDS); + } + + // Create and return the service + return new RestAdapter.Builder() + .setRequestInterceptor(spinnakerRequestInterceptor) + .setEndpoint(front50Endpoint) + .setClient(new Ok3Client(builder.build())) + .setLogLevel(retrofitLogLevel) + .setLog(new RetrofitSlf4jLog(Front50Service.class)) + .setConverter(new JacksonConverter(mapper)) + .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance()) + .build() + .create(Front50Service.class); + } + + @Bean + public ApplicationListener dependentPipelineExecutionListenerAdapter( + DependentPipelineExecutionListener delegate, ExecutionRepository repository) { + return new ExecutionListenerAdapter(delegate, repository); + } +} diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy index 267290afaa..18f8dee3cc 100644 --- a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy +++ b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/config/Front50ConfigurationSpec.groovy @@ -21,6 +21,8 @@ import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties import retrofit.Endpoint import spock.lang.Specification import spock.lang.Subject +import spock.lang.Unroll + import java.util.concurrent.TimeUnit /** @@ -66,35 +68,31 @@ class Front50ConfigurationSpec extends Specification { /** * Verifies that hasCustomTimeouts correctly identifies when custom timeouts exist */ + @Unroll def "hasCustomTimeouts should return #expected when #description"() { given: def props = new Front50ConfigurationProperties.OkHttpConfigurationProperties() - if (connectTimeout != null) { - props.connectTimeoutMs = connectTimeout - } - if (readTimeout != null) { - props.readTimeoutMs = readTimeout - } + props.connectTimeoutMs = connectTimeout + props.readTimeoutMs = readTimeout expect: props.hasCustomTimeouts() == expected where: - description | connectTimeout | readTimeout || expected - "using default values" | 5000L | 120000L || false - "only connect timeout changed" | 10000L | 120000L || true - "only read timeout changed" | 5000L | 30000L || true - "both timeouts changed" | 10000L | 30000L || true + connectTimeout | readTimeout | expected | description + 5000L | 120000L | false | "using default values" + 10000L | 120000L | true | "only connect timeout changed" + 5000L | 30000L | true | "only read timeout changed" + 10000L | 30000L | true | "both timeouts changed" } /** - * This test examines the source code of Front50Configuration to verify that - * the timeout fallback pattern is correctly implemented. This method was chosen - * because OkHttpClient.Builder is a final class and cannot be easily mocked. + * This test verifies that the Front50Configuration implementation follows the correct pattern + * for timeout fallback by examining the source code. */ def "front50Service uses the correct timeout fallback pattern"() { given: - def sourceFile = new File("/Users/shlomodaari/armory/spinnaker-oss-services/orca/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/config/Front50Configuration.groovy") + def sourceFile = new File("/Users/shlomodaari/armory/spinnaker-oss-services/orca/orca-front50/src/main/java/com/netflix/spinnaker/orca/front50/config/Front50Configuration.java") def sourceCode = sourceFile.exists() ? sourceFile.text : null expect: @@ -105,9 +103,9 @@ class Front50ConfigurationSpec extends Specification { sourceCode.contains("builder.readTimeout(okHttpClientConfigurationProperties.getReadTimeoutMs(), TimeUnit.MILLISECONDS)") // Then conditionally override with Front50-specific timeouts if defined - sourceCode.contains("if (front50ConfigurationProperties.okhttp?.connectTimeoutMs != null) {") - sourceCode.contains("builder.connectTimeout(front50ConfigurationProperties.okhttp.connectTimeoutMs, TimeUnit.MILLISECONDS)") - sourceCode.contains("if (front50ConfigurationProperties.okhttp?.readTimeoutMs != null) {") - sourceCode.contains("builder.readTimeout(front50ConfigurationProperties.okhttp.readTimeoutMs, TimeUnit.MILLISECONDS)") + sourceCode.contains("front50ConfigurationProperties.getOkhttp() != null && front50ConfigurationProperties.getOkhttp().getConnectTimeoutMs() != null") + sourceCode.contains("builder.connectTimeout(front50ConfigurationProperties.getOkhttp().getConnectTimeoutMs(), TimeUnit.MILLISECONDS)") + sourceCode.contains("front50ConfigurationProperties.getOkhttp() != null && front50ConfigurationProperties.getOkhttp().getReadTimeoutMs() != null") + sourceCode.contains("builder.readTimeout(front50ConfigurationProperties.getOkhttp().getReadTimeoutMs(), TimeUnit.MILLISECONDS)") } } From 109c125404192cfe05aaa322a9878db2f4937007 Mon Sep 17 00:00:00 2001 From: shlomodaari Date: Wed, 23 Apr 2025 12:45:57 -0700 Subject: [PATCH 8/8] chore: Remove accidentally committed local config file --- orca-web/config/orca-local.yml | 34 ---------------------------------- 1 file changed, 34 deletions(-) delete mode 100644 orca-web/config/orca-local.yml diff --git a/orca-web/config/orca-local.yml b/orca-web/config/orca-local.yml deleted file mode 100644 index e91717dfe7..0000000000 --- a/orca-web/config/orca-local.yml +++ /dev/null @@ -1,34 +0,0 @@ -server: - port: 8083 - -redis: - enabled: false # Disable Redis dependency for local testing - -sql: - enabled: true # Use SQL backend instead - connectionPool: - jdbcUrl: jdbc:h2:mem:orca;MODE=MYSQL;DB_CLOSE_DELAY=-1 - driver: org.h2.Driver - -spring: - datasource: - url: jdbc:h2:mem:orca;MODE=MYSQL;DB_CLOSE_DELAY=-1 - driver-class-name: org.h2.Driver - -keiko: - queue: - memory: - enabled: true # Use in-memory queue - -services: - echo: - enabled: false # Disable echo service dependency for testing - front50: - enabled: false # Disable front50 service dependency for testing - clouddriver: - enabled: false # Disable clouddriver service dependency for testing - -# Debug logging for our fix -logging: - level: - com.netflix.spinnaker.orca.echo.spring.EchoNotifyingStageListener: DEBUG