[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-09 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4787 Is there any problem with the current implementation? The current implementation was carefully done to gracefully handle concurrent removals and allow to pick whether to clean directories with

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-09 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/4787 I was looking into [FLINK-6615](https://issues.apache.org/jira/browse/FLINK-6615), and I think 6615 is not a problem due to files may be created concurrently. But I found the current impl of `dele

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-13 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4787 Fair enough. Seems there are tests for the behavior already, so +1 to merge this Merging... ---

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-13 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4787 Actually, going back here. I would like to not merge this after all. The reason being that in my test run, I found that this does not handle concurrent deletes correctly after all: https://tr

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-13 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4787 I think here is an interesting example of why I am often hesitant with cleanup refactorings, unless there is a pressing need to clean up. It is very hard to judge if the cleaned up versio

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-15 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/4787 @StephanEwen Thanks for letting me know. I'll close this PR ---

[GitHub] flink issue #4787: [FLINK-6615][core] simplify FileUtils

2017-10-15 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/4787 @bowenli86 I hope you don't mind that I pushed back a bit. It's my job to be careful about such things... ---