You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by eastcirclek <gi...@git.apache.org> on 2016/04/28 15:04:33 UTC
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
GitHub user eastcirclek opened a pull request:
https://github.com/apache/flink/pull/1945
[FLINK-3776] Flink Scala shell does not allow to set configuration fo…
The current version of FlinkShell creates a new Configuration object when creating a LocalFlinkMiniCluster in FlinkShell.fetchConnectionInfo().
Instead of creating a new one, FlinkShell just needs to get a configuration object which was already created when GlobalConfiguration.loadConfiguration() is called (which is before FlinkShell.fetchConnectionInfo() is called).
Therefore, just one line modification figures out this issue as shown in this pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/eastcirclek/flink FLINK-3776
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1945.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 #1945
----
commit 3debae60917fd656c23a7221456a44b20fb81d71
Author: eastcirclek <ea...@gmail.com>
Date: 2016-04-28T12:48:59Z
[FLINK-3776] Flink Scala shell does not allow to set configuration for local execution
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/1945#discussion_r62857420
--- Diff: flink-scala-shell/src/main/scala/org/apache/flink/api/scala/FlinkShell.scala ---
@@ -141,7 +141,7 @@ object FlinkShell {
): (String, Int, Option[Either[FlinkMiniCluster, AbstractFlinkYarnCluster]]) = {
config.executionMode match {
case ExecutionMode.LOCAL => // Local mode
- val config = new Configuration()
+ val config = GlobalConfiguration.getConfiguration()
--- End diff --
I think the conflagration needs to be loaded similarly as in line 183.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218987472
`GlobalConfiguration` doesn't ensure that the config has been loaded when you call `get()`. It will give you an empty `Configuration` if you do not call `loadConfiguration` explicitly. If you pass the config after you called the load method, it is clear that the config has been loaded.
Your code works, I'll will just open a follow-up issue to make GlobalConfiguration more explicit, i.e. fail on `get()` if the config hasn't been loaded explicitly.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1945
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218736435
Looks good. Do you think we could make this more explicit by passing the loaded configuration object to the `fetchConnectionInfo` method? That way we don't depend on global variables and we load the config only once.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218476654
Thanks for the pull request. Looks good. Have you tested that the configuration is loaded correctly?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by eastcirclek <gi...@git.apache.org>.
Github user eastcirclek commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218992214
Okay, I got the idea.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218993072
Thanks for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by eastcirclek <gi...@git.apache.org>.
Github user eastcirclek commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218927497
Seems fine but it seems to go against the design of ```GlobalConfiguration``.
The singleton ```GlobalConfiguration``` object is already get and used in that way on few places.
How 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-3776] Flink Scala shell does not allow ...
Posted by eastcirclek <gi...@git.apache.org>.
Github user eastcirclek commented on the pull request:
https://github.com/apache/flink/pull/1945#issuecomment-218727955
@mxm
Yes, I tested it by simply assigning a non-existent directory to taskmanager.tmp.dirs in $FLINK_CONF_DIR/flink-conf.yaml which is read and parsed by GlobalConfiguration.loadConfiguration().
I got the following error messages:
~/flink/flink-dist/target/flink-1.1-SNAPSHOT-bin/flink-1.1-SNAPSHOT/bin$ start-scala-shell.sh local
Starting Flink Shell:
Exception in thread "main" java.io.IOException: Temporary file directory /tmp/east does not exist.
at org.apache.flink.runtime.taskmanager.TaskManager$$anonfun$checkTempDirs$1.apply(TaskManager.scala:2162)
at org.apache.flink.runtime.taskmanager.TaskManager$$anonfun$checkTempDirs$1.apply(TaskManager.scala:2157)
at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:108)
at org.apache.flink.runtime.taskmanager.TaskManager$.checkTempDirs(TaskManager.scala:2157)
at org.apache.flink.runtime.taskmanager.TaskManager$.startTaskManagerComponentsAndActor(TaskManager.scala:1752)
at org.apache.flink.runtime.minicluster.LocalFlinkMiniCluster.startTaskManager(LocalFlinkMiniCluster.scala:142)
at org.apache.flink.runtime.minicluster.FlinkMiniCluster$$anonfun$3.apply(FlinkMiniCluster.scala:319)
at org.apache.flink.runtime.minicluster.FlinkMiniCluster$$anonfun$3.apply(FlinkMiniCluster.scala:312)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.immutable.Range.foreach(Range.scala:141)
at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
at scala.collection.AbstractTraversable.map(Traversable.scala:105)
at org.apache.flink.runtime.minicluster.FlinkMiniCluster.start(FlinkMiniCluster.scala:312)
at org.apache.flink.runtime.minicluster.FlinkMiniCluster.start(FlinkMiniCluster.scala:269)
at org.apache.flink.api.scala.FlinkShell$.fetchConnectionInfo(FlinkShell.scala:148)
at org.apache.flink.api.scala.FlinkShell$.liftedTree1$1(FlinkShell.scala:187)
at org.apache.flink.api.scala.FlinkShell$.startShell(FlinkShell.scala:186)
at org.apache.flink.api.scala.FlinkShell$.main(FlinkShell.scala:134)
at org.apache.flink.api.scala.FlinkShell.main(FlinkShell.scala)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---