[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread mxm
Github user mxm commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r41488791
  
--- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/RemoteEnvironment.java ---
@@ -55,21 +59,8 @@
/** Optional shutdown hook, used in session mode to eagerly terminate 
the last session */
private Thread shutdownHook;
 
-   /**
-* Creates a new RemoteEnvironment that points to the master 
(JobManager) described by the
-* given host name and port.
-* 
-* Each program execution will have all the given JAR files in its 
classpath.
-* 
-* @param host The host name or address of the master (JobManager), 
where the program should be executed.
-* @param port The port of the master (JobManager), where the program 
should be executed. 
-* @param jarFiles The JAR files with code that needs to be shipped to 
the cluster. If the program uses
-* user-defined functions, user-defined input formats, 
or any libraries, those must be
-* provided in the JAR files.
-*/ 
-   public RemoteEnvironment(String host, int port, String... jarFiles) {
--- End diff --

Could we keep this constructor?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146460197
  
+1 for removing the factory methods from ExecutionEnvironment. Most users 
won't even think about using this feature and be rather confused by the 
additional methods. Let's also keep the old constructor of the 
RemoteEnvironment in addition to the new one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146509420
  
@twalthr Doesn't seem to compile :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread asfgit
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146553988
  
Merged. Congrats :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-08 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146555847
  
Thanks, @mxm ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146121012
  
The build succeeded. How do we proceed? It would be very nice if we finally 
merge this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread mxm
Github user mxm commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r41376580
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManager.java
 ---
@@ -124,7 +129,13 @@ public void registerTask(JobID jobId, 
ExecutionAttemptID task, Collection

[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146159521
  
I went through the pull request again and it looks good.

+1 for merging


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread twalthr
Github user twalthr commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r41379357
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManager.java
 ---
@@ -124,7 +129,13 @@ public void registerTask(JobID jobId, 
ExecutionAttemptID task, Collection

[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146192177
  
Do we need another committers +1? It touches very sensitive parts...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146201360
  
I think @StephanEwen wanted to merge it as well. @rmetzger concerns should 
be out of the way with the new version. I would merge this by the end of the 
day.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-07 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-146206122
  
Looks good to merge from my side.

The only uncertainty remaining is whether we need the static factory 
methods on the ExecutionEnvironment. Adding these global classpaths seems a 
very specific case, in which we can probably expect people to manually create a 
RemoteEnvironment. That way we don't over load the ExecutionEnvironment with 
too many factory methods (we have a tendency to get API bloat).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145974916
  
@mxm The issue is probably related to Scala 2.10, since the two passing 
builds have `Dscala-2.11`. I'll investigate tonight if I can get a hold on the 
PR commits (GitHub is somewhat slow at the moment) 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145957532
  
Thanks a lot @twalthr for rebasing the pull request. There is an issue with 
the Scalashell test. Not sure why it only occurs in some builds.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145983873
  
I think the NullPointerException was due to a wrong array to list 
conversion. I don't know why this is related to the Scala version.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145984551
  
It could be that the test is not executed for Scala 2.11.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145838136
  
It took me one working day to rebase it (actually I had to touch almost 
every line again), but now it is mergable. @mxm maybe you could have look at it 
again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-06 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145863259
  
 and it has conflicts again :(


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-02 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-145026274
  
This looks good to merge. However, there are a lot of merge conflicts. 
Could you rebase to the master again? We're are thinking about creating a 
release candidate for 0.10 until the end of next week.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-10-01 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-14463
  
Thanks for updating the JIRA description and the pull request! I'll try to 
review the pull request later today.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-30 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-144438426
  
@rmetzger Your test program runs now. I forgot the support for packaged 
programs. There are quite a lot possiblilities to run a job ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-30 Thread twalthr
Github user twalthr commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40804255
  
--- Diff: 
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java 
---
@@ -47,6 +47,11 @@
"Class with the program entry point (\"main\" method or 
\"getPlan()\" method. Only needed if the " +
"JAR file does not specify the class in 
its manifest.");
 
+   static final Option CLASSPATH_OPTION = new Option("C", "classpath", 
true, "Adds a URL to each user code classloader " +
--- End diff --

@tillrohrmann I added a comment that only protocols are supported which are 
supported by the `URLClassLoader`. The parameter above is Java standard.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-30 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-11498
  
@mxm I updated the JIRA description with the most important comments of the 
discussion.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-26 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40493024
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/classloading/ClassLoaderITCase.java
 ---
@@ -51,12 +50,25 @@ public void testJobWithCustomInputFormat() {
 
PackagedProgram inputSplitTestProg = new 
PackagedProgram(new File(INPUT_SPLITS_PROG_JAR_FILE),
new String[] { 
INPUT_SPLITS_PROG_JAR_FILE,
+   
"", // classpath

"localhost",

String.valueOf(port),

"4" // parallelism
} );

inputSplitTestProg.invokeInteractiveModeForExecution();
 
+
+
+   String classpath = new 
File(INPUT_SPLITS_PROG_JAR_FILE).toURI().toURL().toString();
+   PackagedProgram inputSplitTestProg2 = new 
PackagedProgram(new File(INPUT_SPLITS_PROG_JAR_FILE),
+   new String[] { "",
+   
classpath, // classpath
+   
"localhost",
+   
String.valueOf(port),
+   
"4" // parallelism
+   } );
+   
inputSplitTestProg2.invokeInteractiveModeForExecution();
+
--- End diff --

Looks like I wrote this comment before testing the functionality ;) It 
seems that the test does not detect the issues I found with manual testing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-26 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-143427388
  
Just to defent @twalthr and @aalexandrov here a bit: The JIRA and PR were 
both created before we asked for more extensive descriptions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-25 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-143153638
  
I've read through the JIRA conversation again. We recently had a discussion 
on the mailing list that we would like to have more self-explanatory JIRA 
descriptions. I think this is a classic case where this is simply not met. 
@twalthr Could you please update the JIRA/Pull request description and clearly 
state your problem definition and use case.

These things aside, I think this is a pretty neat idea. Could you rebase 
again to the current master? It also needs to have some more test exposure. It 
seems like the URLs are not properly passed for streaming applications. I might 
be able to help you with rebasing or tests. Just let me know how you want to 
proceed and where you are currently. Thank you.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread twalthr
Github user twalthr commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40303043
  
--- Diff: 
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java 
---
@@ -47,6 +47,11 @@
"Class with the program entry point (\"main\" method or 
\"getPlan()\" method. Only needed if the " +
"JAR file does not specify the class in 
its manifest.");
 
+   static final Option CLASSPATH_OPTION = new Option("C", "classpath", 
true, "Adds a URL to each user code classloader " +
--- End diff --

HDFS is not possible yet. the user would have to provide a special URL 
protocol handler via `java -Djava.protocol.handler.pkgs=org.my.protocols` such 
that the URLClassloader understands the protocol.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread tillrohrmann
Github user tillrohrmann commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40317151
  
--- Diff: 
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java 
---
@@ -47,6 +47,11 @@
"Class with the program entry point (\"main\" method or 
\"getPlan()\" method. Only needed if the " +
"JAR file does not specify the class in 
its manifest.");
 
+   static final Option CLASSPATH_OPTION = new Option("C", "classpath", 
true, "Adds a URL to each user code classloader " +
--- End diff --

Is this documented somewhere?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142944429
  
Do we have to have a separate list of classpaths in addition to the user 
classloader or can we have a user classloader which contains all the JARs that 
a user has provided (either via the user jar or via an extra folder)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142943052
  
I just had a look at this pull request. Why don't we treat the JARs in the 
given folder like the the other jar files that are submitted to the cluster via 
the BlobManager as part of the job submission? This assumes that the Client can 
access the JARs but that's IMHO a fair assumption to make. If the Client can't 
access one of the provided JAR files, we can fail with an appropriate error 
message.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142960455
  
@mxm The jars in which folder? The main motivation point of the pull 
request is to add the option to add a classpaths where one can generate code at 
runtime.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142961440
  
@mxm Think of it that the URLs are http URLs to an http server that serves 
dynamically generated classes...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-24 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142975593
  
@aalexandrov Concerning the folders, you wrote:
> The idea is to add support for proper *folders* next to jars when opening 
an execution environment. Since folders cannot be handled the same way as jars 
during execution (i.e., serialize and ship them around by the blob manager), 
the assumption for folder paths is that they are accessible from all agents in 
the distributed runtime (JobManager + TaskManagers), e.g., via shared NFS 
folders.

@StephanEwen Why can't the "dynamically generated" class files not be 
shipped during deployment of the job and thereafter be available through the 
flink user code classloader?

I didn't see any hint in the JIRA or in this pull request that the classes 
should be generated at runtime. So either I don't understand this feature 
correctly or the feature description in the JIRA is not correct.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-23 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40179781
  
--- Diff: 
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java 
---
@@ -47,6 +47,11 @@
"Class with the program entry point (\"main\" method or 
\"getPlan()\" method. Only needed if the " +
"JAR file does not specify the class in 
its manifest.");
 
+   static final Option CLASSPATH_OPTION = new Option("C", "classpath", 
true, "Adds a URL to each user code classloader " +
--- End diff --

would `hdfs:///` also be possible?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-23 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/593#discussion_r40180582
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/classloading/ClassLoaderITCase.java
 ---
@@ -51,12 +50,25 @@ public void testJobWithCustomInputFormat() {
 
PackagedProgram inputSplitTestProg = new 
PackagedProgram(new File(INPUT_SPLITS_PROG_JAR_FILE),
new String[] { 
INPUT_SPLITS_PROG_JAR_FILE,
+   
"", // classpath

"localhost",

String.valueOf(port),

"4" // parallelism
} );

inputSplitTestProg.invokeInteractiveModeForExecution();
 
+
+
+   String classpath = new 
File(INPUT_SPLITS_PROG_JAR_FILE).toURI().toURL().toString();
+   PackagedProgram inputSplitTestProg2 = new 
PackagedProgram(new File(INPUT_SPLITS_PROG_JAR_FILE),
+   new String[] { "",
+   
classpath, // classpath
+   
"localhost",
+   
String.valueOf(port),
+   
"4" // parallelism
+   } );
+   
inputSplitTestProg2.invokeInteractiveModeForExecution();
+
--- End diff --

good test!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-23 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142527214
  
https://github.com/apache/flink/pull/858 is merged, so we can try to get 
consensus here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-23 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-142547078
  
-1 

I was not able to get this feature working as expected, with the following 
job:

```java
public class URLClassloader {

public static void main(String[] args) throws Exception {
URL about = URLClassloader.class.getResource("/about.md");

System.out.println("about = "+about);

final StreamExecutionEnvironment see = 
StreamExecutionEnvironment.getExecutionEnvironment();
see.addSource(new SourceFunction() {
@Override
public void run(SourceContext ctx) throws 
Exception {
URL about = 
this.getClass().getResource("/about.md");
System.out.println("about = "+about);
}
@Override
public void cancel() {}
});

see.execute();
}
}
```

invoking it like this
`./bin/flink run -c com.dataartisans.URLClassloader -C 
file:///home/robert/flink-workdir/flink-irc-logs.github.io/ 
/home/robert/flink-workdir/debug-project-0.10/target/debug-project-0.10-1.0-SNAPSHOT.jar`

lead to the following output

```
./bin/flink run -c com.dataartisans.URLClassloader -C 
file:///home/robert/flink-workdir/flink-irc-logs.github.io/ 
/home/robert/flink-workdir/debug-project-0.10/target/debug-project-0.10-1.0-SNAPSHOT.jar
about = file:/home/robert/flink-workdir/flink-irc-logs.github.io/about.md
09/23/2015 11:38:33 Job execution switched to status RUNNING.
09/23/2015 11:38:33 Custom Source(1/1) switched to SCHEDULED 
09/23/2015 11:38:33 Custom Source(1/1) switched to DEPLOYING 
09/23/2015 11:38:33 Custom Source(1/1) switched to RUNNING 
09/23/2015 11:38:33 Custom Source(1/1) switched to FINISHED 
09/23/2015 11:38:33 Job execution switched to status FINISHED.
robert@robert-da ~/incubator-flink/build-target 
(git)-[CustomUrlsToUserClassloader] % cat 
log/flink-robert-jobmanager-0-robert-da.out
about = null
about = null
about = null
about = null
about = null
```

So it seems that the client is using the correct classpath, and its able to 
load the file. The JobManager/TaskManager (I've started flink with 
start-local.sh) doesn't seem to be able to load the file.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-21 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-141983001
  
I agree. Lets wait until https://github.com/apache/flink/pull/858 is 
merged. As far as I understood Stephan, this PR depends on #858.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-21 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-141950905
  
I rebased the PR two times. So before I will rebase it a third time, it 
would be great if we finally find a consensus.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-18 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-141457169
  
Do you mean the 0.10-milestone-1 release? or the final 0.10 release?

Fabian is managing the 0.10-milestone-1 release, as far as I know he has 
not started with the release activities, so we could still include it, given 
that we have consensus to merge the PR 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-18 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-141455406
  
@twalthr @rmetzger is there a chance to include this PR in the 10.0 release?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-09-18 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-141459804
  
@rmetzger the final 10.0, this and the Scala 2.11 compatibility are the two 
pending issues that make the current Emma master incompatible with vanilla 
Flink.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-07-27 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-125359065
  
I have rebased the PR and added commits of @aalexandrov that introduce a 
CLI option `--classpath`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-06-04 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-108839757
  
I was originally thinking to merge this after #681 , but I may have to 
merge this by itself to get it into the release.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-05-18 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-103067431
  
I have implemented your comments and rebased to the current master. The 
build succeeded except for a KafkaITCase, but I think that has nothing to do 
with my code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-05-18 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-103068570
  
Oh .. failing {{KafkaITCase}}s are something I'm very interested in ;)
Sadly I can not see the root cause of the issue :( But the issue is 
unrelated to your change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-27 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-96599424
  
Yes, user-facing remains strings which are file paths.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-27 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-96591270
  
It is not a problem in exposing this to the client as well, I was just 
unsure if it is necessary. I will add to the client as well to be in sync.
Yes, it makes sense to use URLs instead of  URLs and Files. But the user 
facing APIs remain Strings right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-27 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-96532907
  
Yes, this would make things a lot cleaner. @twalthr what do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-24 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95868495
  
I think that keeping the classpaths in sync is really crucial. What is the 
problem in exposing this to the client as well?

Can we make the following assumptions for the use case where we need the 
global class path:
  - The URL is either a file path that points to a directory accessible to 
all nodes (NFS or so) and the client runs in the cluster as well.
  - The URL is an HTTP URL or so that points to a file server that serves 
the classes to work in non-shared directory settings.

Some other remark: This makes the code quite dirty on a lot of parts, since 
we have always Files and URLs in Client, executors, ... Can we not store 
everything as URLs (rather than having Strings for jar files and URLs extra). 
Files can be expressed as URLs anyways as well...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-24 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95845500
  
OK, @StephanEwen, any thoughts on this? Should we allow that the local user 
code class loader in the client potentially doesn't have the same jars 
available as the workers?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-23 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95500680
  
This does not work if the user uses classes that are not available on the 
local machine since you don't add the additional class path entries in 
JobWithJars.buildUserCodeClassLoader(). Correct?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-23 Thread aljoscha
Github user aljoscha commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95564844
  
Yes, this is true, but the way it is implemented, the folders are not 
always added to the class loader. Maybe I'm wrong here, but 
JobWithJars.getUserCodeClassLoader and JobWithJars.buildUserCodeClassLoader 
don't add the URLs to the ClassLoader that they create.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-23 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95591759
  
@aljoscha Yes, this is what I also mentioned in my PR description. If we 
are using a RemoteEnvironment, it is unlikely that the classpath that is 
available on the cluster is also valid on the local machine. The user has to 
add the libraries to his local project manually.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-23 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-95559261
  
@aljoscha I'm sorry, I cannot follow? Can you elaborate?

The idea is to add support for proper folders next to jars when opening an 
execution environment. Since folders cannot be handled the same way as jars 
during execution (i.e., serialize and ship them around by the blob manager), 
the assumption for folder paths is that they are accessible from all agents in 
the distributed runtime (JobManager + TaskManagers), e.g., via shared NFS 
folders.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-14 Thread aalexandrov
Github user aalexandrov commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-92841182
  
:clap:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-14 Thread twalthr
Github user twalthr commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-92805121
  
It builds now ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-13 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/593#issuecomment-92451748
  
I didn't spot anything. But, the travis build didn't pass ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1789] [core] [runtime] [java-api] Allow...

2015-04-13 Thread twalthr
GitHub user twalthr opened a pull request:

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

[FLINK-1789] [core] [runtime] [java-api] Allow adding of URLs to the 
usercode class loader

See [FLINK-1789].

I was a little bit unsure in `JobWithJars` because it is not guaranteed 
that the classpath is correct locally, maybe someone can have a look at it.

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

$ git pull https://github.com/twalthr/flink CustomUrlsToUserClassloader

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

https://github.com/apache/flink/pull/593.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 #593


commit 1f45400fb0bfa3388b5f641cc700a6f15bc40678
Author: twalthr twal...@apache.org
Date:   2015-04-08T23:33:40Z

[FLINK-1789] [core] [runtime] [java-api] Allow adding of URLs to the 
usercode class loader




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---