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
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
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
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
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
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
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
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
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
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
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 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
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
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 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
---
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
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
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
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
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
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
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
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
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
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
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.
---
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
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
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.
---
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.
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.
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
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 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
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
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
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
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
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.
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
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
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
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
60 matches
Mail list logo
