[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-19 Thread ASF GitHub Bot (JIRA) 

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

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

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

https://github.com/apache/flink/pull/1904#discussion_r60238192
  
--- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/RemoteEnvironment.java ---
@@ -133,26 +137,31 @@ public RemoteEnvironment(String host, int port, 
Configuration clientConfig,
this.port = port;
this.clientConfiguration = clientConfig == null ? new 
Configuration() : clientConfig;
if (jarFiles != null) {
-   this.jarFiles = new URL[jarFiles.length];
+   this.jarFiles = new ArrayList(jarFiles.length);
for (int i = 0; i < jarFiles.length; i++) {
try {
-   this.jarFiles[i] = new 
File(jarFiles[i]).getAbsoluteFile().toURI().toURL();
+   this.jarFiles.add(new 
File(jarFiles[i]).getAbsoluteFile().toURI().toURL());
} catch (MalformedURLException e) {
throw new IllegalArgumentException("JAR 
file path invalid", e);
}
}
}
else {
-   this.jarFiles = null;
+   this.jarFiles = Collections.emptyList();
+   }
+
+   if (globalClasspaths == null) {
+   this.globalClasspaths = Collections.emptyList();
+   } else {
+   this.globalClasspaths = Arrays.asList(globalClasspaths);
}
-   this.globalClasspaths = globalClasspaths;
}
 
// 

 
@Override
public JobExecutionResult execute(String jobName) throws Exception {
-   ensureExecutorCreated();
+   PlanExecutor executor = getExecutor();
--- End diff --

You're right, it's bad that the `PlanExecutor` is not stopped after it has 
been used. I will fix this by checking in `ScalaShellRemoteEnvironment` whether 
`this.executor` is set. If true, then it will call `this.executor.stop()`. That 
way, there will always be at most one `PlanExecutor` active and the last one is 
stopped by the `dispose` call.


> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-19 Thread ASF GitHub Bot (JIRA) 

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

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

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

https://github.com/apache/flink/pull/1904#discussion_r60235985
  
--- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/RemoteEnvironment.java ---
@@ -133,26 +137,31 @@ public RemoteEnvironment(String host, int port, 
Configuration clientConfig,
this.port = port;
this.clientConfiguration = clientConfig == null ? new 
Configuration() : clientConfig;
if (jarFiles != null) {
-   this.jarFiles = new URL[jarFiles.length];
+   this.jarFiles = new ArrayList(jarFiles.length);
--- End diff --

Will fix it.


> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-19 Thread ASF GitHub Bot (JIRA) 

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

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

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

https://github.com/apache/flink/pull/1904#discussion_r60231836
  
--- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/RemoteEnvironment.java ---
@@ -133,26 +137,31 @@ public RemoteEnvironment(String host, int port, 
Configuration clientConfig,
this.port = port;
this.clientConfiguration = clientConfig == null ? new 
Configuration() : clientConfig;
if (jarFiles != null) {
-   this.jarFiles = new URL[jarFiles.length];
+   this.jarFiles = new ArrayList(jarFiles.length);
--- End diff --

Nitpick - No need of  :( 


> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-19 Thread ASF GitHub Bot (JIRA) 

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

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

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

https://github.com/apache/flink/pull/1904#discussion_r60217702
  
--- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/RemoteEnvironment.java ---
@@ -133,26 +137,31 @@ public RemoteEnvironment(String host, int port, 
Configuration clientConfig,
this.port = port;
this.clientConfiguration = clientConfig == null ? new 
Configuration() : clientConfig;
if (jarFiles != null) {
-   this.jarFiles = new URL[jarFiles.length];
+   this.jarFiles = new ArrayList(jarFiles.length);
for (int i = 0; i < jarFiles.length; i++) {
try {
-   this.jarFiles[i] = new 
File(jarFiles[i]).getAbsoluteFile().toURI().toURL();
+   this.jarFiles.add(new 
File(jarFiles[i]).getAbsoluteFile().toURI().toURL());
} catch (MalformedURLException e) {
throw new IllegalArgumentException("JAR 
file path invalid", e);
}
}
}
else {
-   this.jarFiles = null;
+   this.jarFiles = Collections.emptyList();
+   }
+
+   if (globalClasspaths == null) {
+   this.globalClasspaths = Collections.emptyList();
+   } else {
+   this.globalClasspaths = Arrays.asList(globalClasspaths);
}
-   this.globalClasspaths = globalClasspaths;
}
 
// 

 
@Override
public JobExecutionResult execute(String jobName) throws Exception {
-   ensureExecutorCreated();
+   PlanExecutor executor = getExecutor();
--- End diff --

would it make sense to move the assignment to ```this.executor``` from 
```getExecutor()``` to this line? as it stands the ScalaShellRemoteEnvironment 
executor is never closed.


> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-18 Thread ASF GitHub Bot (JIRA) 

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

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

GitHub user tillrohrmann opened a pull request:

https://github.com/apache/flink/pull/1904

[FLINK-3774] [shell] Forwards Flink configuration to PlanExecutor

Thanks for contributing to Apache Flink. Before you open your pull request, 
please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your 
pull request. For more information and/or questions please refer to the [How To 
Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful 
description of your changes.

- [X] General
  - The pull request references the related JIRA issue
  - The pull request addresses only one issue
  - Each commit in the PR has a meaningful commit message

- [X] Documentation
  - Documentation has been added for new functionality
  - Old documentation affected by the pull request has been updated
  - JavaDoc for public methods has been added

- [X] Tests & Build
  - Functionality added by the pull request is covered by tests
  - `mvn clean verify` has been executed successfully locally or a Travis 
build has passed

The ScalaShellRemoteEnvironment did not properly forward the given Flink 
configuration
to the PlanExecutor. Consequently, it was not possible to configure the 
Client to connect
to an HA cluster. This PR corrects the forwarding.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tillrohrmann/flink fixScalaShell

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1904.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1904


commit 427fe789dcd20ba94dc9cebcef85b629ada6ce9c
Author: Till Rohrmann 
Date:   2016-04-18T15:41:47Z

[FLINK-3774] [shell] Forwards Flink configuration to PlanExecutor

The ScalaShellRemoteEnvironment did not properly forward the given Flink 
configuration
to the PlanExecutor. Consequently, it was not possible to configure the 
Client to connect
to an HA cluster. This PR corrects the forwarding.




> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Assignee: Andrea Sella
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (FLINK-3774) Flink configuration is not correctly forwarded to PlanExecutor in ScalaShellRemoteEnvironment

2016-04-18 Thread Stefano Baghino (JIRA) 

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

Stefano Baghino commented on FLINK-3774:


I've tested it on the 1.0 as well and the issue is there as well.

> Flink configuration is not correctly forwarded to PlanExecutor in 
> ScalaShellRemoteEnvironment
> -
>
> Key: FLINK-3774
> URL: https://issues.apache.org/jira/browse/FLINK-3774
> Project: Flink
>  Issue Type: Bug
>  Components: Scala Shell
>Affects Versions: 1.1.0
>Reporter: Till Rohrmann
>Priority: Minor
> Fix For: 1.1.0
>
>
> Currently, the {{ScalaShellRemoteEnvironment}} does not correctly forwards 
> the Flink configuration to the {{PlanExecutor}}. Therefore, it is not 
> possible to use the Scala shell in combination with an HA cluster which needs 
> the configuration parameters set in the configuration.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)