[ 
https://issues.apache.org/jira/browse/FLINK-8339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16311706#comment-16311706
 ] 

ASF GitHub Bot commented on FLINK-8339:
---------------------------------------

Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5225#discussion_r159706464
  
    --- Diff: 
flink-clients/src/test/java/org/apache/flink/client/cli/DefaultCLITest.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.client.cli;
    +
    +import org.apache.flink.client.deployment.ClusterDescriptor;
    +import org.apache.flink.client.program.ClusterClient;
    +import org.apache.flink.configuration.Configuration;
    +import org.apache.flink.configuration.JobManagerOptions;
    +import org.apache.flink.util.TestLogger;
    +
    +import org.apache.commons.cli.CommandLine;
    +import org.junit.Assert;
    +import org.junit.Rule;
    +import org.junit.Test;
    +import org.junit.rules.TemporaryFolder;
    +
    +import java.net.InetSocketAddress;
    +
    +/**
    + * Tests for the {@link DefaultCLI}.
    + */
    +public class DefaultCLITest extends TestLogger {
    +
    +   @Rule
    +   public TemporaryFolder temporaryFolder = new TemporaryFolder();
    +
    +   /**
    +    * Tests that the configuration is properly passed via the DefaultCLI 
to the
    +    * created ClusterDescriptor.
    +    */
    +   @Test
    +   public void testConfigurationPassing() throws Exception {
    +           final DefaultCLI defaultCLI = new DefaultCLI();
    +
    +           final String configurationDirectory = 
temporaryFolder.newFolder().getAbsolutePath();
    +           final String[] args = {};
    +
    +           CommandLine commandLine = 
defaultCLI.parseCommandLineOptions(args, false);
    +
    +           final String localhost = "localhost";
    +           final int port = 1234;
    +           final Configuration configuration = new Configuration();
    +
    +           configuration.setString(JobManagerOptions.ADDRESS, localhost);
    +           configuration.setInteger(JobManagerOptions.PORT, port);
    +
    +           final InetSocketAddress expectedAddress = new 
InetSocketAddress(localhost, port);
    +
    +           final ClusterDescriptor<?> clusterDescriptor = 
defaultCLI.createClusterDescriptor(
    +                   configuration,
    +                   configurationDirectory,
    +                   commandLine);
    +
    +           final ClusterClient clusterClient = 
clusterDescriptor.retrieve(defaultCLI.getClusterId(
    +                   configuration,
    +                   commandLine));
    +
    +           Assert.assertEquals(expectedAddress, 
clusterClient.getJobManagerAddress());
    +   }
    +
    +   /**
    +    * Tests that command line options override the configuration settings.
    +    */
    +   @Test
    +   public void testManualConfigurationOverride() throws Exception {
    +           final DefaultCLI defaultCLI = new DefaultCLI();
    +
    +           final String manualHostname = "123.123.123.123";
    +           final int manualPort = 4321;
    +           final String configurationDirectory = 
temporaryFolder.newFolder().getAbsolutePath();
    +           final String[] args = {"-m", manualHostname + ':' + manualPort};
    +
    +           CommandLine commandLine = 
defaultCLI.parseCommandLineOptions(args, false);
    +
    +           final String localhost = "localhost";
    +           final int port = 1234;
    +           final Configuration configuration = new Configuration();
    +
    +           configuration.setString(JobManagerOptions.ADDRESS, localhost);
    +           configuration.setInteger(JobManagerOptions.PORT, port);
    +
    +           final InetSocketAddress expectedAddress = new 
InetSocketAddress(manualHostname, manualPort);
    --- End diff --
    
    nit: variable is declared too early. Will be used at the `assertEquals`.


> Let CustomCommandLine return a ClusterDescriptor
> ------------------------------------------------
>
>                 Key: FLINK-8339
>                 URL: https://issues.apache.org/jira/browse/FLINK-8339
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Client
>    Affects Versions: 1.5.0
>            Reporter: Till Rohrmann
>            Assignee: Till Rohrmann
>              Labels: flip-6
>             Fix For: 1.5.0
>
>
> The {{CustomCommandLine}} currently is able to retrieve a {{ClusterClient}} 
> and deploy a cluster. In order to better separate concerns it would be good 
> if the {{CustomCommandLine}} would simply return a {{ClusterDescriptor}} 
> which could then be used to retrieve a {{ClusterClient}} or to deploy a Flink 
> cluster. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to