[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15474137#comment-15474137
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2450
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15473197#comment-15473197
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
Rebasing on the latest master and letting Travis run. If it gives green
light, I will merge the PR.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15464917#comment-15464917
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
I've re-introduced the `ForkableFlinkMiniCluster`. However, this time it
only starts the normal distributed component classes (e.g. `JobManager` instead
of `TestingJobManager`, etc.). This implies that the testing messages are no
longer supported when using the `ForkableFlinkMiniCluster`.
I think the user should not meddle with the testing components in the first
place. Therefore, it seems to be fair to offer the user a mini cluster to run
his tests but not one which instantiates testing components. The recommended
cluster is the `LocalFlinkMiniCluster` from now on. However, in order to not
break all existing test code, we still maintain the `ForkableFlinkMiniCluster`.
Not having to start the testing classes allows to move them back into the
test scope of `flink-runtime`, since they are no longer required by the
`flink-test-utils` module.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15464765#comment-15464765
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
I will re-introduce the `ForkableFlinkMiniCluster` which is simply
extending the `LocalFlinkMiniCluster`.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15464700#comment-15464700
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
Yes that is true. However, the `flink-test-utils` have not been guaranteed
to be stable if I'm not mistaken. The changes one has to apply is to replace
the `ForkableFlinkMiniCluster` with the `LocalFlinkMiniCluster`.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15464553#comment-15464553
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2450
Quick question: The `ForkableFlinkMiniCluster` was something semi-public,
in the sense that it was part of the `flink-test-utils` project and used by
quite a few users. So this would be a breaking change.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15458876#comment-15458876
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
Builds are passing on Travis (own repository). Would like to merge this PR
if there are no objections.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15458128#comment-15458128
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
Will rebase on the latest master and #2449 to run the travis tests again.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15456048#comment-15456048
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2450
Thanks for the review @StephanEwen.
I agree that we've different style preferences concerning method
parameters. I also agree that the one parameter per line is a little bit more
verbose and requires more vertical scrolling. The best approach would probably
be to reduce the number of method parameters by grouping them.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15455997#comment-15455997
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2450#discussion_r77212855
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/minicluster/FlinkMiniCluster.scala
---
@@ -69,7 +69,7 @@ abstract class FlinkMiniCluster(
ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY,
InetAddress.getByName("localhost").getHostAddress())
- val configuration = generateConfiguration(userConfiguration)
+ protected val _configuration = generateConfiguration(userConfiguration)
--- End diff --
It's actually the original configuration which will usually have the job
manager ipc port set to 0. Thus, in order to return a configuration which
points to the current leader, we have to generate a new configuration via
`configuration` which inserts the selected port. I'll rename the variable to
`originalConfiguration`.
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15455536#comment-15455536
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2450
I think these changes are good.
As a very personal and biased statement: I think the code overdoes it a bit
with the "every parameter on a new line" policy. We have beautiful wide screens
and I get sore fingers from scrolling up and down again to figure out enough
context of a code passage ;-)
Also, the style breaks the "visual" separation between different parts of
statement, like "members of a return tuple" vs. "function parameters", or
"clauses of an if statement" and "body of the if statement". I know the idea of
that pattern was to improve code readability, but I am getting the feeling this
is approaching a "verschlimmbesserung" (an improvement that makes things worse).
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLINK-4458) Remove ForkableFlinkMiniCluster
[
https://issues.apache.org/jira/browse/FLINK-4458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15455509#comment-15455509
]
ASF GitHub Bot commented on FLINK-4458:
---
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/2450#discussion_r77180730
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/minicluster/FlinkMiniCluster.scala
---
@@ -69,7 +69,7 @@ abstract class FlinkMiniCluster(
ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY,
InetAddress.getByName("localhost").getHostAddress())
- val configuration = generateConfiguration(userConfiguration)
+ protected val _configuration = generateConfiguration(userConfiguration)
--- End diff --
Why the underscore here?
> Remove ForkableFlinkMiniCluster
> ---
>
> Key: FLINK-4458
> URL: https://issues.apache.org/jira/browse/FLINK-4458
> Project: Flink
> Issue Type: Improvement
> Components: Tests
>Reporter: Till Rohrmann
>Assignee: Till Rohrmann
>Priority: Minor
>
> After addressing FLINK-4424 we should be able to get rid of the
> {{ForkableFlinkMiniCluster}} since we no longer have to pre-determine a port
> in Flink. Thus, by setting the ports to {{0}} and letting the OS choose a
> free port, there should no longer be conflicting port requests. Consequently,
> the {{ForkableFlinkMiniCluster}} will become obsolete.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
