XComp commented on code in PR #22794:
URL: https://github.com/apache/flink/pull/22794#discussion_r1231055996
##########
flink-dist/src/main/resources/flink-conf.yaml:
##########
@@ -16,6 +16,7 @@
# limitations under the License.
Review Comment:
```suggestion
# limitations under the License.
```
I never noticed the wrong indentation here :exploding_head:
##########
flink-connectors/flink-connector-files/pom.xml:
##########
@@ -34,6 +34,13 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config><!--
+ chill ArraysAsListSerializer
Review Comment:
We need to add this because `ArrayAsListSerializer` is used within
`com.twitter:chill-java:jar:0.7.6` which is a test dependency within
`flink-java`? Therefore, any module that depends on `flink-java` in its test
code has to add this part to the surefire configuration?!
##########
flink-dist/src/main/resources/flink-conf.yaml:
##########
@@ -16,6 +16,7 @@
# limitations under the License.
################################################################################
+env.java.opts.all: --add-exports=java.base/sun.net.util=ALL-UNNAMED
--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
--add-exports=java.security.jgss/sun.security.krb5=ALL-UNNAMED
--add-opens=java.base/java.lang=ALL-UNNAMED
--add-opens=java.base/java.net=ALL-UNNAMED
--add-opens=java.base/java.io=ALL-UNNAMED
--add-opens=java.base/java.lang.reflect=ALL-UNNAMED
--add-opens=java.base/java.text=ALL-UNNAMED
--add-opens=java.base/java.time=ALL-UNNAMED
--add-opens=java.base/java.util=ALL-UNNAMED
--add-opens=java.base/java.util.concurrent=ALL-UNNAMED
--add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED
--add-opens=java.base/java.util.concurrent.locks=ALL-UNNAMED
Review Comment:
Could we add a comment on the selection of exclusion here as well?
##########
flink-core/pom.xml:
##########
@@ -33,6 +33,25 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config> <!--
+ required by JmxServer
+ -->--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
<!--
+ PluginConfigTest
+ -->--add-opens=java.base/java.util=ALL-UNNAMED <!--
Review Comment:
Is this necessary because of the `import java.util.Map`? That would be odd
because we would probably have to add it to almost any module. :thinking:
##########
flink-core/pom.xml:
##########
@@ -33,6 +33,25 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config> <!--
+ required by JmxServer
+ -->--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
<!--
+ PluginConfigTest
+ -->--add-opens=java.base/java.util=ALL-UNNAMED <!--
+ ExceptionUtilsTest
+ -->--add-opens=java.base/java.lang=ALL-UNNAMED <!--
+ -->--add-opens=java.base/java.lang.invoke=ALL-UNNAMED
<!--
+ StateDescriptorTest
+ -->--add-opens=java.base/java.io=ALL-UNNAMED <!--
+ SerializersTest
+ -->--add-opens=java.base/java.net=ALL-UNNAMED <!--
Review Comment:
`java.net` doesn't occur in `SerializersTest` :thinking:
##########
flink-core/pom.xml:
##########
@@ -33,6 +33,25 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config> <!--
+ required by JmxServer
+ -->--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
<!--
+ PluginConfigTest
+ -->--add-opens=java.base/java.util=ALL-UNNAMED <!--
+ ExceptionUtilsTest
+ -->--add-opens=java.base/java.lang=ALL-UNNAMED <!--
+ -->--add-opens=java.base/java.lang.invoke=ALL-UNNAMED
<!--
+ StateDescriptorTest
+ -->--add-opens=java.base/java.io=ALL-UNNAMED <!--
+ SerializersTest
+ -->--add-opens=java.base/java.net=ALL-UNNAMED <!--
+ InitOutputPathTest
+
-->--add-opens=java.base/java.util.concurrent.locks=ALL-UNNAMED <!--
Review Comment:
wouldn't that be rather a `--add-exports` because we're using it only for
extending the class? :thinking:
##########
flink-core/pom.xml:
##########
@@ -33,6 +33,25 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config> <!--
+ required by JmxServer
+ -->--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
<!--
+ PluginConfigTest
+ -->--add-opens=java.base/java.util=ALL-UNNAMED <!--
+ ExceptionUtilsTest
+ -->--add-opens=java.base/java.lang=ALL-UNNAMED <!--
+ -->--add-opens=java.base/java.lang.invoke=ALL-UNNAMED
<!--
Review Comment:
I didn't find any occurrences of `invoke` in `ExceptionUtilsTest` :thinking:
##########
flink-core/pom.xml:
##########
@@ -33,6 +33,25 @@ under the License.
<packaging>jar</packaging>
+ <properties>
+ <surefire.module.config> <!--
+ required by JmxServer
+ -->--add-exports=java.rmi/sun.rmi.registry=ALL-UNNAMED
<!--
+ PluginConfigTest
+ -->--add-opens=java.base/java.util=ALL-UNNAMED <!--
+ ExceptionUtilsTest
+ -->--add-opens=java.base/java.lang=ALL-UNNAMED <!--
+ -->--add-opens=java.base/java.lang.invoke=ALL-UNNAMED
<!--
+ StateDescriptorTest
+ -->--add-opens=java.base/java.io=ALL-UNNAMED <!--
Review Comment:
because of `import.io.File`? :thinking:
##########
.mvn/jvm.config:
##########
@@ -1 +1,7 @@
-XX:+IgnoreUnrecognizedVMOptions
+--add-exports=java.security.jgss/sun.security.krb5=ALL-UNNAMED
Review Comment:
Can't we add a comment here as well about the reasons for the selection here?
##########
flink-python/pyflink/pyflink_gateway_server.py:
##########
@@ -254,8 +254,10 @@ def launch_gateway_server_process(env, args):
[construct_flink_classpath(env), construct_hadoop_classpath(env)])
if "FLINK_TESTING" in env:
classpath = os.pathsep.join([classpath,
construct_test_classpath()])
- command = [java_executable, jvm_args,
"-XX:+IgnoreUnrecognizedVMOptions"] + jvm_opts \
- + log_settings + ["-cp", classpath, program_args.main_class] +
program_args.other_args
+ command = [java_executable, jvm_args,
"-XX:+IgnoreUnrecognizedVMOptions",
+ "--add-opens=jdk.proxy2/jdk.proxy2=ALL-UNNAMED"] \
Review Comment:
could we add a comment for the reason here as well?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
