sjwiesman edited a comment on pull request #110: URL: https://github.com/apache/flink-statefun/pull/110#issuecomment-628974226
Thank you for working on this feature! I haven't taken a look at the code, Igal or Gordon are more qualified to look at this part of the code base, but I wanted to provide some early feedback. 1) I am fairly certain that flinks shaded netty, which is transitively available in statefun-flink-core via flink-streaming-java, contains a unix socket implementation. Does it make sense to use that instead of pulling in a new dependency? I'm not sure the tradeoffs here. 2) If we do add a new dependency to statefun-flink-core we need to update the NOTICE file accordingly[1]. Can you please check how junixsocket-core is licensed? 3) You can take a look at flink-e2e-tests module to see how to write integration tests. They are dockerized and the project provides some utilities to simplify things. Do note that #108 is refactoring the utility classes so it might make sense to wait for that to be merged in first. [1] https://github.com/apache/flink-statefun/blob/master/statefun-flink/statefun-flink-core/src/main/resources/META-INF/NOTICE ---------------------------------------------------------------- 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: us...@infra.apache.org
