wangyang0918 commented on a change in pull request #14891:
URL: https://github.com/apache/flink/pull/14891#discussion_r580845485
##########
File path:
flink-clients/src/test/java/org/apache/flink/client/deployment/application/ClassPathPackagedProgramRetrieverTest.java
##########
@@ -315,6 +327,24 @@ public void
testRetrieveCorrectUserClasspathsWithSpecifiedEntryClass()
containsInAnyOrder(expectedURLs.stream().map(URL::toString).toArray()));
}
+ @Test
Review comment:
I think the new introduced test does not cover the merging with usrlib
and `pipeline.classpaths`. What do you think about making the test like this?
```
@Test
public void testRetrieveCorrectUserClasspathsWithPipelineClasspaths()
throws IOException, FlinkException, ProgramInvocationException {
final Configuration configuration = new Configuration();
final List<String> pipelineJars =
Arrays.asList("file:///path/of/p1.jar",
"http://path/of/p2.jar";);
configuration.set(PipelineOptions.CLASSPATHS, pipelineJars);
final ClassPathPackagedProgramRetriever retrieverUnderTest =
ClassPathPackagedProgramRetriever.newBuilder(PROGRAM_ARGUMENTS)
.setUserLibDirectory(userDirHasEntryClass)
.setJarFile(TestJob.getTestJobJar())
.setConfiguration(configuration)
.build();
final JobGraph jobGraph = retrieveJobGraph(retrieverUnderTest, new
Configuration());
final List<URL> expectedMergedURLs = new ArrayList<>(expectedURLs);
expectedMergedURLs.addAll(
pipelineJars.stream()
.map(FunctionUtils.uncheckedFunction(URL::new))
.collect(Collectors.toList()));
assertThat(
jobGraph.getClasspaths().stream().map(URL::toString).collect(Collectors.toList()),
containsInAnyOrder(expectedMergedURLs.stream().map(URL::toString).toArray()));
}
```
##########
File path:
flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneApplicationClusterEntryPoint.java
##########
@@ -62,16 +62,16 @@ public static void main(String[] args) {
args,
new
StandaloneApplicationClusterConfigurationParserFactory(),
StandaloneApplicationClusterEntryPoint.class);
+ Configuration configuration =
loadConfigurationFromClusterConfig(clusterConfiguration);
Review comment:
Could be `final`.
----------------------------------------------------------------
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]
