jfrazee commented on a change in pull request #4183:
URL: https://github.com/apache/nifi/pull/4183#discussion_r580571040



##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -638,8 +637,12 @@ public void setUpClient(final ProcessContext context) 
throws IOException, Unreco
         }
 
         // Set timeouts
-        
okHttpClientBuilder.connectTimeout((context.getProperty(PROP_CONNECT_TIMEOUT).asTimePeriod(TimeUnit.MILLISECONDS).intValue()),
 TimeUnit.MILLISECONDS);
-        
okHttpClientBuilder.readTimeout(context.getProperty(PROP_READ_TIMEOUT).asTimePeriod(TimeUnit.MILLISECONDS).intValue(),
 TimeUnit.MILLISECONDS);
+        
okHttpClientBuilder.connectTimeout((context.getProperty(PROP_CONNECT_TIMEOUT).evaluateAttributeExpressions().asTimePeriod(TimeUnit.MILLISECONDS).intValue()),
 TimeUnit.MILLISECONDS);
+
+        if 
(context.getProperty(PROP_READ_TIMEOUT).isExpressionLanguagePresent())
+            okHttpClientBuilder.addInterceptor(new TimeoutInterceptor());

Review comment:
       See my comment on `newBuilder()` below. I think it's right that it's not 
sufficient to set it on the primary builder b/c you want to use per-flowfile 
attributes, but I think the static timeout + an interceptor is problematic.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to