[GitHub] flink issue #5357: [hotfix][JobGraph] Eliminate the conditions of parallelis...

2018-02-07 Thread maqingxiang
Github user maqingxiang commented on the issue: https://github.com/apache/flink/pull/5357 Thanks for your review @zentol I got it. ---

[GitHub] flink issue #5357: [hotfix][JobGraph] Eliminate the conditions of parallelis...

2018-02-07 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5357 @maqingxiang You are correct that the parallelism check is redundant, but that doesn't automatically mean that it should be removed. This change imo hurts readability as the basic parallelism

[GitHub] flink issue #5357: [hotfix][JobGraph] Eliminate the conditions of parallelis...

2018-02-02 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5357 In this case, could you please close the PR? (we cannot easily do that) ---

[GitHub] flink issue #5357: [hotfix][JobGraph] Eliminate the conditions of parallelis...

2018-01-29 Thread maqingxiang
Github user maqingxiang commented on the issue: https://github.com/apache/flink/pull/5357 Thanks for your review @zentol As far as I know, If the partitioner of the edge is forward, then their parallelism must be the same. Maybe I'm completely missing something... ---

[GitHub] flink issue #5357: [hotfix][JobGraph] Eliminate the conditions of parallelis...

2018-01-29 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/5357 I would be in favor of closing this PR. This change doesn't _really_ improve anything, but removes a simple (and intuitive) sanity check. ---