You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/10/13 19:44:34 UTC

[GitHub] [flink-statefun] igalshilman opened a new pull request #276: [FLINK-24534] Use the async transport if transport is not specified.

igalshilman opened a new pull request #276:
URL: https://github.com/apache/flink-statefun/pull/276


   This PR sets the Netty based, asynchronous transport as the default transport
   for endpoint definitions that did not specify a particular transport.
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] igalshilman merged pull request #276: [FLINK-24534] Use the async transport if transport is not specified.

Posted by GitBox <gi...@apache.org>.
igalshilman merged pull request #276:
URL: https://github.com/apache/flink-statefun/pull/276


   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] sjwiesman commented on pull request #276: [FLINK-24534] Use the async transport if transport is not specified.

Posted by GitBox <gi...@apache.org>.
sjwiesman commented on pull request #276:
URL: https://github.com/apache/flink-statefun/pull/276#issuecomment-942714405


   Please update the docs as well. 
   
   Should we increase the default max number of async requests? 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] sjwiesman commented on a change in pull request #276: [FLINK-24534] Use the async transport if transport is not specified.

Posted by GitBox <gi...@apache.org>.
sjwiesman commented on a change in pull request #276:
URL: https://github.com/apache/flink-statefun/pull/276#discussion_r729895780



##########
File path: statefun-flink/statefun-flink-core/src/main/java/org/apache/flink/statefun/flink/core/nettyclient/NettyClient.java
##########
@@ -94,6 +91,24 @@ public static NettyClient from(
     return new NettyClient(shared, eventLoop, pool, endpoint, headers, totalRequestBudgetInNanos);
   }
 
+  private static SslContext sslContext(
+      NettySharedResources shared, NettyRequestReplySpec spec, Endpoint endpoint) {
+    if (!endpoint.useTls()) {
+      return null;
+    }
+    if (spec.trustStorePath.isEmpty()) {
+      return shared.sslContext();
+    }
+    SslContextBuilder builder = SslContextBuilder.forClient();
+    try {
+      File trustCertCollectionFile = new File(spec.trustStorePath);
+      builder.trustManager(trustCertCollectionFile);
+      return builder.build();
+    } catch (SSLException e) {
+      throw new IllegalStateException("Unable to setup an SSL context.");
+    }
+  }

Review comment:
       Because I can look into my crystal ball and see the ML questions already from people who mistyped the path to their trust store ....
   
   ```suggestion
     private static SslContext sslContext(
         NettySharedResources shared, NettyRequestReplySpec spec, Endpoint endpoint) {
       if (!endpoint.useTls()) {
         return null;
       }
   
       if (spec.trustStorePath.isEmpty()) {
         return shared.sslContext();
       }
   
       File trustCertCollectionFile = new File(spec.trustStorePath);
       if (!trustCertCollectionFile.exists()) {
         throw new IllegalStateException(
             String.format(
                 "Unable to setup an SSL context, invalid path `%s`."
                     + "Please ensure the trust store file exists and is mounted locally on each TaskManager.",
                 spec.trustStorePath));
       }
   
       SslContextBuilder builder = SslContextBuilder.forClient();
       try {
         builder.trustManager(trustCertCollectionFile);
         return builder.build();
       } catch (SSLException e) {
         throw new IllegalStateException("Unable to setup an SSL context.", e);
       }
     }
   ```




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org