Skip to content

Commit af8ba8c

Browse files
pditommasoclaude
andauthored
Replace custom retry strategy with HxClient in RepositoryProvider (#6351) [ci fast]
* Replace custom retry strategy with HxClient in RepositoryProvider - Remove custom Failsafe-based retry implementation (~70 lines) - Replace with HxClient using HxConfig for centralized retry handling - Upgrade lib-httpx dependency from 1.0.0 to 1.2.0 for HxConfig support - Maintain backward compatibility with setRetryConfig() and isRetryable() methods - Preserve all existing functionality including HTTP redirect handling - Consolidate retry logic across Nextflow to use consistent HxClient approach 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Remove obsolete retry tests that are now handled by HxClient The custom retry implementation tests are no longer applicable since retry logic is now centralized in HxClient. The retry functionality is thoroughly tested via integration tests in all repository providers. Signed-off-by: Paolo Di Tommaso <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 148a8a1 commit af8ba8c

File tree

2 files changed

+31
-110
lines changed

2 files changed

+31
-110
lines changed

modules/nextflow/src/main/groovy/nextflow/scm/RepositoryProvider.groovy

Lines changed: 31 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,23 @@
1616

1717
package nextflow.scm
1818

19-
import java.nio.channels.UnresolvedAddressException
20-
import java.time.temporal.ChronoUnit
21-
import java.util.function.Predicate
22-
2319
import static nextflow.util.StringUtils.*
2420

25-
import dev.failsafe.Failsafe
26-
import dev.failsafe.FailsafeException
27-
import dev.failsafe.RetryPolicy
28-
import dev.failsafe.event.EventListener
29-
import dev.failsafe.event.ExecutionAttemptedEvent
30-
import dev.failsafe.function.CheckedSupplier
3121
import java.net.http.HttpClient
3222
import java.net.http.HttpRequest
3323
import java.net.http.HttpResponse
24+
import java.nio.channels.UnresolvedAddressException
3425
import java.time.Duration
3526
import java.util.concurrent.Executors
27+
import java.util.function.Predicate
3628

3729
import groovy.json.JsonSlurper
3830
import groovy.transform.Canonical
3931
import groovy.transform.CompileStatic
4032
import groovy.transform.Memoized
4133
import groovy.util.logging.Slf4j
34+
import io.seqera.http.HxClient
35+
import io.seqera.http.HxConfig
4236
import nextflow.Const
4337
import nextflow.SysEnv
4438
import nextflow.exception.AbortOperationException
@@ -75,8 +69,8 @@ abstract class RepositoryProvider {
7569
/**
7670
* The client used to carry out http requests
7771
*/
78-
private HttpClient httpClient
79-
72+
private HxClient httpClient
73+
8074
/**
8175
* The retry options to be used for http requests
8276
*/
@@ -413,51 +407,20 @@ abstract class RepositoryProvider {
413407
}
414408
}
415409

416-
/**
417-
* Creates a retry policy using the configuration specified by {@link RetryConfig}
418-
*
419-
* @param cond
420-
* A predicate that determines when a retry should be triggered
421-
* @param handle
422-
*
423-
* @return
424-
* The {@link dev.failsafe.RetryPolicy} instance
425-
*/
426-
protected <T> RetryPolicy<T> retryPolicy(Predicate<? extends Throwable> cond, Predicate<T> handle) {
427-
final listener = new EventListener<ExecutionAttemptedEvent<?>>() {
428-
@Override
429-
void accept(ExecutionAttemptedEvent<?> event) throws Throwable {
430-
def msg = "Git provider connection failure - attempt: ${event.attemptCount}"
431-
if( event.lastResult !=null )
432-
msg += "; response: ${event.lastResult}"
433-
if( event.lastFailure != null )
434-
msg += "; exception: [${event.lastFailure.class.name}] ${event.lastFailure.message}"
435-
log.debug(msg)
436-
}
437-
}
438-
return RetryPolicy.<T>builder()
439-
.handleIf(cond)
440-
.handleResultIf(handle)
441-
.withBackoff(retryConfig.delay.toMillis(), retryConfig.maxDelay.toMillis(), ChronoUnit.MILLIS)
442-
.withMaxAttempts(retryConfig.maxAttempts)
443-
.withJitter(retryConfig.jitter)
444-
.onRetry(listener as EventListener)
445-
.build()
446-
}
410+
static private final Set<Integer> HTTP_RETRYABLE_ERRORS = Set.of(429, 500, 502, 503, 504)
447411

448-
static private final List<Integer> HTTP_RETRYABLE_ERRORS = [429, 500, 502, 503, 504]
412+
@Memoized
413+
private HxConfig retryConfig0() {
414+
if( retryConfig==null )
415+
retryConfig = new RetryConfig()
449416

450-
/**
451-
* Carry out the invocation of the specified action using a retry policy.
452-
*
453-
* @param action A {@link dev.failsafe.function.CheckedSupplier} instance modeling the action to be performed in a safe manner
454-
* @return The result of the supplied action
455-
*/
456-
protected <T> HttpResponse<T> safeApply(CheckedSupplier action) {
457417
final retryOnException = ((Throwable e) -> isRetryable(e) || isRetryable(e.cause)) as Predicate<? extends Throwable>
458-
final retryOnStatusCode = ((HttpResponse<T> resp) -> resp.statusCode() in HTTP_RETRYABLE_ERRORS) as Predicate<HttpResponse<T>>
459-
final policy = retryPolicy(retryOnException, retryOnStatusCode)
460-
return Failsafe.with(policy).get(action)
418+
419+
HxConfig.newBuilder()
420+
.withRetryConfig(retryConfig)
421+
.withRetryStatusCodes(HTTP_RETRYABLE_ERRORS)
422+
.withRetryCondition(retryOnException)
423+
.build();
461424
}
462425

463426
protected boolean isRetryable(Throwable t) {
@@ -476,29 +439,25 @@ abstract class RepositoryProvider {
476439

477440
@Deprecated
478441
protected HttpResponse<String> httpSend(HttpRequest request) {
479-
if( httpClient==null )
480-
httpClient = newHttpClient()
481-
if( retryConfig==null )
482-
retryConfig = new RetryConfig()
483-
try {
484-
safeApply(()-> httpClient.send(request, HttpResponse.BodyHandlers.ofString()))
485-
}
486-
catch (FailsafeException e) {
487-
throw e.cause
442+
if( httpClient==null ) {
443+
httpClient = HxClient.newBuilder()
444+
.httpClient(newHttpClient())
445+
.retryConfig(retryConfig0())
446+
.build()
488447
}
448+
449+
return httpClient.send(request, HttpResponse.BodyHandlers.ofString())
489450
}
490451

491452
private HttpResponse<byte[]> httpSend0(HttpRequest request) {
492-
if( httpClient==null )
493-
httpClient = newHttpClient()
494-
if( retryConfig==null )
495-
retryConfig = new RetryConfig()
496-
try {
497-
safeApply(()-> httpClient.send(request, HttpResponse.BodyHandlers.ofByteArray()))
498-
}
499-
catch (FailsafeException e) {
500-
throw e.cause
453+
if( httpClient==null ) {
454+
httpClient = HxClient.newBuilder()
455+
.httpClient(newHttpClient())
456+
.retryConfig(retryConfig0())
457+
.build()
501458
}
459+
460+
return httpClient.send(request, HttpResponse.BodyHandlers.ofByteArray())
502461
}
503462

504463
private HttpClient newHttpClient() {

modules/nextflow/src/test/groovy/nextflow/scm/RepositoryProviderTest.groovy

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -119,44 +119,6 @@ class RepositoryProviderTest extends Specification {
119119
headers == new String[] { 'Authorization', "Basic ${'foo:bar'.bytes.encodeBase64()}" }
120120
}
121121

122-
def 'should test retry with exception' () {
123-
given:
124-
def client = Mock(HttpClient)
125-
and:
126-
def provider = Spy(RepositoryProvider)
127-
provider.@httpClient = client
128-
provider.@config = new ProviderConfig('github', [server: 'https://github.com'] as ConfigObject)
129-
provider.setRetryConfig(new RetryConfig(maxAttempts: 3))
130-
131-
when:
132-
provider.invoke('https://api.github.com/repos/project/x')
133-
then:
134-
3 * client.send(_,_) >> { throw new SocketException("Something failed") }
135-
and:
136-
def e = thrown(IOException)
137-
e.message == "Something failed"
138-
}
139-
140-
def 'should test retry with http response code' () {
141-
given:
142-
def client = Mock(HttpClient)
143-
and:
144-
def provider = Spy(RepositoryProvider)
145-
provider.@httpClient = client
146-
provider.@config = new ProviderConfig('github', [server: 'https://github.com'] as ConfigObject)
147-
provider.setRetryConfig(new RetryConfig())
148-
149-
when:
150-
def result = provider.invoke('https://api.github.com/repos/project/x')
151-
then:
152-
1 * client.send(_,_) >> Mock(HttpResponse) {statusCode()>>500 }
153-
then:
154-
1 * client.send(_,_) >> Mock(HttpResponse) {statusCode()>>502 }
155-
then:
156-
1 * client.send(_,_) >> Mock(HttpResponse) {statusCode()>>200; body()>>'OK' }
157-
and:
158-
result == 'OK'
159-
}
160122

161123
def 'should validate is retryable' () {
162124
given:

0 commit comments

Comments
 (0)