You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by pnowojski <gi...@git.apache.org> on 2017/09/29 13:09:47 UTC
[GitHub] flink pull request #4749: [FLINK-7739][tests] Properly shutdown resources in...
GitHub user pnowojski opened a pull request:
https://github.com/apache/flink/pull/4749
[FLINK-7739][tests] Properly shutdown resources in tests
This is a fixup of tests, without touching the production code.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pnowojski/flink kafka-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4749.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 #4749
----
commit 4e55087d091993a9552b5f732abab8a5c0a5a014
Author: Piotr Nowojski <pi...@gmail.com>
Date: 2017-09-29T12:45:28Z
[FLINK-7739][kafka-tests] Shutdown NetworkFailureProxy
commit a5bbc6fdf2a3e039739a0392dad3d61791f0c3fd
Author: Piotr Nowojski <pi...@gmail.com>
Date: 2017-09-29T12:50:37Z
[FLINK-7739][tests] Properly close flink mini cluster
----
---
[GitHub] flink issue #4749: [FLINK-7739][tests] Properly shutdown resources in tests
Posted by pnowojski <gi...@git.apache.org>.
Github user pnowojski commented on the issue:
https://github.com/apache/flink/pull/4749
Users shouldn't be able to call `shutdown()` and if they are, they shouldn't be using it (it would be incorrect). `shutdown()` was a protected method and shouldn't be used the users. Unfortunately in many places in `flink-tests` it was used instead of public `stop()`, because protected methods are visible inside the same package and it was really confusing which one should be used.
Ideally it would be great to have another visibility level like "only visible to the children", but there is no such thing :/ Thus I renamed `shutdown()` to make clear distinction from public `stop()`.
---
[GitHub] flink issue #4749: [FLINK-7739][tests] Properly shutdown resources in tests
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/4749
I think this looks good.
Since it rename the mini cluster methods (meaning it does actually change user facing pre-production code which should also be stable), I would only merge this for 1.4 and not 1.3.x.
Given the frequent confusion between `stop()` and `shutdown()`, I think the (renamed) methods should have proper docs before merging this.
---
[GitHub] flink pull request #4749: [FLINK-7739][tests] Properly shutdown resources in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4749
---