[GitHub] flink issue #6365: [FLINK-9849] [hbase] Hbase upgrade to 2.0.1

2018-07-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/6365 Should this be a separate connector for HBase 2.x where we would also keep a connector for HBase 1.x? ---

[GitHub] flink issue #6378: [FLINK-9236] [pom] upgrade the version of apache parent p...

2018-07-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/6378 Any reason not to use version 20 or are we just being conservative? ---

[GitHub] flink issue #5776: [FLINK-8708] Unintended integer division in StandaloneThr...

2018-03-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5776 LGTM but I'd cc @StephanEwen since the code looks to have been ported recently. ---

[GitHub] flink pull request #5731: [FLINK-9033][config] Replace usages of deprecated ...

2018-03-27 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5731#discussion_r177536243 --- Diff: flink-scala-shell/src/test/scala/org/apache/flink/api/scala/ScalaShellITCase.scala --- @@ -23,7 +23,7 @@ import java.io._ import

[GitHub] flink pull request #5676: [FLINK-8910][tests] Automated end-to-end test for ...

2018-03-27 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5676#discussion_r177468202 --- Diff: flink-end-to-end-tests/src/main/java/org/apache/flink/streaming/tests/StickyAllocationAndLocalRecoveryTestJob.java --- @@ -0,0 +1,451

[GitHub] flink issue #5488: [FLINK-8335] [hbase] Upgrade hbase connector dependency t...

2018-03-27 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5488 I haven't looked at the changelog but HBase 1.4.2 was released a few weeks ago. ---

[GitHub] flink issue #5731: [FLINK-9033][config] Replace usages of deprecated TASK_MA...

2018-03-26 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5731 Test failures, and needs rebasing. ---

[GitHub] flink pull request #5497: Propose fixing some typos

2018-02-16 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5497#discussion_r168853239 --- Diff: docs/dev/migration.md --- @@ -145,16 +145,16 @@ public class BufferingSink implements SinkFunction<Tuple2<String, I

[GitHub] flink pull request #5497: Propose fixing some typos

2018-02-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5497#discussion_r168616567 --- Diff: docs/dev/migration.md --- @@ -145,16 +145,16 @@ public class BufferingSink implements SinkFunction<Tuple2<String, I

[GitHub] flink issue #5497: Propose fixing some typos

2018-02-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5497 +1 ---

[GitHub] flink pull request #5488: [FLINK-8335] [hbase] Upgrade hbase connector depen...

2018-02-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5488#discussion_r168592172 --- Diff: flink-connectors/flink-hbase/pom.xml --- @@ -34,7 +34,7 @@ under the License. jar - 1.3.1

[GitHub] flink issue #5419: [FLINK-8574][travis] Add timestamp to logging messages

2018-02-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5419 Can we also add timestamps to the surefire messages using `systemPropertyVariables`? https://gualtierotesta.wordpress.com/2015/11/01/tutorial-logging-during-tests/ http

[GitHub] flink issue #5205: [FLINK-8037] Fix integer multiplication or shift implicit...

2018-01-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5205 @StephanEwen of the five occurrences in `MemoryManager`, `ChannelWriterOutputView`, and `InPlaceMutableHashTable` only one occurs in a function that is called elsewhere. How would you write

[GitHub] flink pull request #5205: [FLINK-8037] Fix integer multiplication or shift i...

2018-01-17 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5205#discussion_r162199532 --- Diff: flink-core/src/main/java/org/apache/flink/util/AbstractID.java --- @@ -186,7 +186,7 @@ private static long byteArrayToLong(byte[] ba, int offset

[GitHub] flink pull request #5294: [FLINK-8427] [optimizer] Checkstyle for org.apache...

2018-01-15 Thread greghogan
Github user greghogan closed the pull request at: https://github.com/apache/flink/pull/5294 ---

[GitHub] flink pull request #5294: [FLINK-8427] [optimizer] Checkstyle for org.apache...

2018-01-14 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5294 [FLINK-8427] [optimizer] Checkstyle for org.apache.flink.optimizer.costs ## What is the purpose of the change Enforce checkstyle for org.apache.flink.optimizer.costs You can merge

[GitHub] flink pull request #5292: [FLINK-8422] [core] Checkstyle for org.apache.flin...

2018-01-12 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5292 [FLINK-8422] [core] Checkstyle for org.apache.flink.api.java.tuple ## What is the purpose of the change Update TupleGenerator for Flink's checkstyle and rebuild Tuple and TupleBuilder

[GitHub] flink pull request #5291: [FLINK-8361] [build] Remove create_release_files.s...

2018-01-12 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5291 [FLINK-8361] [build] Remove create_release_files.sh ## What is the purpose of the change The monolithic create_release_files.sh does not support building without Hadoop and has been

[GitHub] flink pull request #5289: [hotfix] [docs] Fix typos

2018-01-12 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5289 [hotfix] [docs] Fix typos From the IntelliJ `Typos` inspection. You can merge this pull request into a Git repository by running: $ git pull https://github.com/greghogan/flink

[GitHub] flink issue #5262: [FLINK-8082][build] Bump flink version for japicmp plugin

2018-01-10 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5262 I'm working on a `CHECKLIST` file for `tools/releasing` to codify these several issues that have come up recently. ---

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160795814 --- Diff: docs/concepts/runtime.md --- @@ -88,40 +94,36 @@ By default, Flink allows subtasks to share slots even if they are subtasks of di

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160793612 --- Diff: docs/concepts/runtime.md --- @@ -88,40 +94,36 @@ By default, Flink allows subtasks to share slots even if they are subtasks of di

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160794201 --- Diff: docs/concepts/runtime.md --- @@ -46,19 +46,23 @@ The Flink runtime consists of two types of processes: - The **JobManagers** (also called

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160795540 --- Diff: docs/concepts/runtime.md --- @@ -88,40 +94,36 @@ By default, Flink allows subtasks to share slots even if they are subtasks of di

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160794027 --- Diff: docs/concepts/runtime.md --- @@ -28,12 +28,12 @@ under the License. ## Tasks and Operator Chains -For distributed execution

[GitHub] flink pull request #5277: [hotfix][docs]Review to reduce passive voice, impr...

2018-01-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5277#discussion_r160793754 --- Diff: docs/concepts/runtime.md --- @@ -28,12 +28,12 @@ under the License. ## Tasks and Operator Chains -For distributed execution

[GitHub] flink pull request #5279: [hotfix] [build] Print cache info

2018-01-10 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5279 [hotfix] [build] Print cache info Print the size of the Maven cache copied for each TravisCI job. You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] flink pull request #5242: [hotfix] Fix typos

2018-01-04 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5242 [hotfix] Fix typos ## What is the purpose of the change Fix typos from the IntelliJ "Typos" inspection. I have tried to preserve British vs American English spellings

[GitHub] flink issue #5112: [FLINK-8175] [DataStream API java/scala] remove flink-str...

2018-01-04 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5112 @bowenli86 Checkstyle is Java-only. There is a [Scalastyle](http://www.scalastyle.org). ---

[GitHub] flink pull request #5213: [Minor][docs][metrics] Fix typo of metrics docs

2018-01-03 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5213#discussion_r159523422 --- Diff: docs/monitoring/metrics.md --- @@ -905,7 +903,7 @@ Thus, in order to infer the metric identifier: Job-/TaskManager

[GitHub] flink pull request #5205: [FLINK-8037] Fix integer multiplication or shift i...

2017-12-22 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5205 [FLINK-8037] Fix integer multiplication or shift implicitly cast to long ## What is the purpose of the change Fixes potential overflow flagged by the IntelliJ inspection "In

[GitHub] flink pull request #5195: [hotfix] [build] Always include Kafka 0.11 connect...

2017-12-20 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5195 [hotfix] [build] Always include Kafka 0.11 connector Now that Flink only supports builds for Scala 2.11+ we can unconditionally enable the Kafka 0.11 connector. You can merge this pull request

[GitHub] flink issue #5180: [FLINK-8292] Remove unnecessary force cast in DataStreamS...

2017-12-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5180 +0 ---

[GitHub] flink pull request #5175: [FLINK-8280][checkstyle] fix checkstyle in BlobSer...

2017-12-20 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5175#discussion_r158056059 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobUtils.java --- @@ -150,8 +150,7 @@ static File initLocalStorageDirectory(String

[GitHub] flink pull request #5156: [FLINK-8079][tests] Stop end-to-end test execution...

2017-12-20 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5156#discussion_r158037581 --- Diff: tools/travis_mvn_watchdog.sh --- @@ -543,35 +543,45 @@ case $TEST in printf "Running end-to-end te

[GitHub] flink issue #5136: [FLINK-8222] [build] Update Scala version

2017-12-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5136 I'll merge this to 1.5 only. ---

[GitHub] flink issue #5126: [FLINK-5506] [gelly] Fix CommunityDetection NullPointerEx...

2017-12-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5126 @StephanEwen thanks for the tip. I'll remove the added `TypeHint` methods and commit to 1.4 and 1.5. ---

[GitHub] flink issue #5157: [hotfix] [docs] Consistent capitalization in Mesos docume...

2017-12-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5157 @joerg84 could you file a PR for this change in `docs/ops/config.md` and elsewhere in `docs/ops/deployment/mesos.md`? ---

[GitHub] flink issue #5138: [FLINK-8217] [Kinesis connector] Properly annotate APIs o...

2017-12-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5138 @bowenli86 the `@Public` annotation is much more than an acknowledgement, it promises that Flink will support that API essentially forever (despite the large number of "2.0" tickets, i

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-12-12 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r156386303 --- Diff: docs/monitoring/metrics.md --- @@ -333,7 +333,7 @@ reporters will be instantiated on each job and task manager when they are starte

[GitHub] flink issue #5134: [FLINK-8220] Implement set of network benchmarks

2017-12-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5134 @StephanEwen thanks for the link and thoughtful discussion. Including `jmh` benchmarks has come up multiple times. Do you see any path to including such a benchmark module in the main Flink repo? ---

[GitHub] flink issue #5144: [Minor][cleanup] Remove unnecessary semicolons

2017-12-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5144 @yew1eb thanks for this PR. It would be helpful if you could provide how you discovered these style issues (IntelliJ analysis?) and if there is a way to automatically discover and/or flag

[GitHub] flink issue #5134: [FLINK-8220] Implement set of network benchmarks

2017-12-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5134 @pnowojski see FLINK-2848 and FLINK-2973. Also the BSD + Patents conversation along with Flink's dependence on Amazon's Kinesis library (likewise [Category X](https://www.apache.org/legal

[GitHub] flink issue #5138: [FLINK-8192] [Kinesis connector] Properly annotate APIs o...

2017-12-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5138 Changes to the public API require a [FLIP](https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals). ---

[GitHub] flink issue #5133: [hotfix] Fix typo in AkkaUtils method

2017-12-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5133 [ERROR] src/main/java/org/apache/flink/runtime/rpc/akka/AkkaRpcServiceUtils.java:[41,8] (imports) UnusedImports: Unused import: java.net.InetSocketAddress. ---

[GitHub] flink issue #5133: [hotfix] Fix typo in AkkaUtils method

2017-12-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5133 @casidiablo please also describe why or how the removed code came to be unused. I see that `StandaloneHaServices#RESOURCE_MANAGER_RPC_ENDPOINT_NAME` was left unused by `433a345e`. ---

[GitHub] flink issue #5072: [FLINK-7984][build] Bump snappy-java to 1.1.4

2017-12-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5072 @yew1eb have you looked at FLINK-6965? ---

[GitHub] flink pull request #5137: [FLINK-8223] [build] Update Hadoop versions

2017-12-07 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5137 [FLINK-8223] [build] Update Hadoop versions ## What is the purpose of the change Update Hadoop minor versions for Flink 1.5 development cycle. ## Brief change log Update

[GitHub] flink pull request #5136: [FLINK-8222] [build] Update Scala version

2017-12-07 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5136 [FLINK-8222] [build] Update Scala version ## What is the purpose of the change This is an incremental upgrade to the Scala security release 2.11.12. "A privilege escal

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-12-07 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r155625966 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/metrics/MetricRegistryImplTest.java --- @@ -76,8 +76,27 @@ public void testIsShutdown

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-12-07 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r155620220 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java --- @@ -108,15 +118,36 @@ public static

[GitHub] flink pull request #5135: [hotfix] [doc] Fix typo in TaskManager and Environ...

2017-12-07 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5135#discussion_r155627163 --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala --- @@ -585,7 +585,7 @@ class TaskManager

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-12-07 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r155618187 --- Diff: docs/monitoring/metrics.md --- @@ -329,11 +329,11 @@ or by assigning unique names to jobs and operators. Metrics can be exposed

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-12-07 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r155623170 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java --- @@ -44,7 +48,13 @@ private static

[GitHub] flink issue #5134: [FLINK-8220] Implement set of network benchmarks

2017-12-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5134 Or if `flink-benchmarks` does not need to be distributed then that code could be contributed to the main Flink repository. ---

[GitHub] flink pull request #5126: [FLINK-5506] [gelly] Fix CommunityDetection NullPo...

2017-12-05 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5126 [FLINK-5506] [gelly] Fix CommunityDetection NullPointerException ## What is the purpose of the change This fixes a regression which can result in `NullPointerException` when running

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-11-30 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r154121821 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java --- @@ -43,8 +46,8 @@ private

[GitHub] flink pull request #5099: [FLINK-8080][metrics] Remove need for "metrics.rep...

2017-11-30 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5099#discussion_r154122694 --- Diff: flink-core/src/main/java/org/apache/flink/configuration/MetricOptions.java --- @@ -25,20 +25,9 @@ public class MetricOptions

[GitHub] flink issue #4574: [FLINK-6864] Fix confusing "invalid POJO type" messages f...

2017-11-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4574 @zjureel am merging this ... thanks for the PR and edits! ---

[GitHub] flink issue #4383: [hotfix] [optimizer] Normalize job plan operator formatti...

2017-11-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4383 @zentol @fhueske I am merging the change with the extra space since this looks to have been the original intent. I've looked at both forms without finding a strong preference. ---

[GitHub] flink issue #4374: repalce map.put with putIfAbsent

2017-11-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4374 @RebornHuan although this change looks to be correct and makes good use of the newer API, there is a trade-off between deleting a line of code called during an error condition and the risk

[GitHub] flink pull request #5076: [FLINK-7574][build] POM Cleanup flink-clients

2017-11-28 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5076#discussion_r153503257 --- Diff: pom.xml --- @@ -891,6 +905,41 @@ under the License

[GitHub] flink pull request #5076: [FLINK-7574][build] POM Cleanup flink-clients

2017-11-28 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5076#discussion_r153500167 --- Diff: pom.xml --- @@ -891,6 +905,41 @@ under the License

[GitHub] flink issue #4666: [FLINK-7613][Documentation] Fixed typographical error

2017-11-28 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4666 @raymondtay are we still looking to make this change? Defining the mapper may be just as likely to confuse new Flink users. Also, when updating future PRs you want to rebase to master

[GitHub] flink issue #5067: [FLINK-8142][config] Cleanup reference to deprecated cons...

2017-11-27 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5067 +1; merging ... ---

[GitHub] flink issue #5047: Code refine of WordWithCount

2017-11-27 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5047 Do we still want to make this change since the PR is now only adding a single comment? ---

[GitHub] flink issue #5061: [hotfix] [docs] Update checkstyle version in documentatio...

2017-11-23 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5061 @zentol thanks, I'll merge and backport. ---

[GitHub] flink pull request #5061: [hotfix] [docs] Update checkstyle version in docum...

2017-11-23 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5061 [hotfix] [docs] Update checkstyle version in documentation ## What is the purpose of the change Update the recommended checkstyle version in the documentation to match the active version

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152608576 --- Diff: docs/concepts/programming-model.md --- @@ -33,53 +33,52 @@ Flink offers different levels of abstraction to develop streaming/batch applicat

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152608405 --- Diff: docs/concepts/programming-model.md --- @@ -132,14 +131,13 @@ One typically distinguishes different types of windows, such as *tumbling window

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152607252 --- Diff: docs/concepts/programming-model.md --- @@ -33,53 +33,52 @@ Flink offers different levels of abstraction to develop streaming/batch applicat

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152612059 --- Diff: docs/concepts/runtime.md --- @@ -107,21 +107,20 @@ With hyper-threading, each slot then takes 2 or more hardware thread contexts

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152605878 --- Diff: docs/concepts/programming-model.md --- @@ -33,53 +33,52 @@ Flink offers different levels of abstraction to develop streaming/batch applicat

[GitHub] flink pull request #5045: [hotfix][docs] Review of concepts docs for grammar...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5045#discussion_r152609451 --- Diff: docs/concepts/runtime.md --- @@ -74,10 +74,10 @@ To control how many tasks a worker accepts, a worker has so called **task slots* Each

[GitHub] flink issue #5047: Code refine of WordWithCount

2017-11-22 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5047 This change doesn't work since `WordWithCount` needs to be a POJO which requires a default or no-args constructor and non-final public attributes or getters/setters. Since the type is not a POJO

[GitHub] flink issue #5049: [FLINK-8081][metrics] Annotate 'MetricRegistry#getReporte...

2017-11-22 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5049 +1 ---

[GitHub] flink issue #5012: [FLINK-8070][yarn][tests] Print errors found in log files

2017-11-22 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5012 Ah, thanks @zentol for the clarification. Very nice to have this! ---

[GitHub] flink pull request #5006: [hotfix][docs][QS] MInor cleanup of QS documentati...

2017-11-22 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5006#discussion_r152591791 --- Diff: docs/dev/stream/state/queryable_state.md --- @@ -162,14 +161,19 @@ So far, you have set up your cluster to run with queryable state and you have

[GitHub] flink issue #4877: [FLINK-4877] About SourceFunction extends Serializable

2017-11-21 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4877 @vim-wj if you are okay with Stephan's suggestion could you close this pull request? Also, a small note: `FLINK-4877` references a [Jira ticket](https://issues.apache.org/jira/browse

[GitHub] flink pull request #5044: [FLINK-8126] [build] Fix and update checkstyle

2017-11-21 Thread greghogan
GitHub user greghogan opened a pull request: https://github.com/apache/flink/pull/5044 [FLINK-8126] [build] Fix and update checkstyle ## What is the purpose of the change Update to the latest checkstyle version and fix the errors not previously detected. ## Brief

[GitHub] flink pull request #4946: [FLINK-7967] [config] Deprecate Hadoop specific Fl...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4946#discussion_r152405696 --- Diff: flink-dist/src/main/resources/flink-conf.yaml --- @@ -151,6 +151,9 @@ jobmanager.web.port: 8081 # Path to the Hadoop configuration

[GitHub] flink pull request #5006: [hotfix][docs][QS] MInor cleanup of QS documentati...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5006#discussion_r152402516 --- Diff: docs/dev/stream/state/queryable_state.md --- @@ -162,14 +161,19 @@ So far, you have set up your cluster to run with queryable state and you have

[GitHub] flink pull request #5006: [hotfix][docs][QS] MInor cleanup of QS documentati...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5006#discussion_r152402801 --- Diff: docs/dev/stream/state/queryable_state.md --- @@ -162,14 +161,19 @@ So far, you have set up your cluster to run with queryable state and you have

[GitHub] flink pull request #5006: [hotfix][docs][QS] MInor cleanup of QS documentati...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5006#discussion_r152402097 --- Diff: docs/dev/stream/state/queryable_state.md --- @@ -60,7 +60,7 @@ The Queryable State feature consists of three main entities: returning

[GitHub] flink issue #5012: [FLINK-8070][yarn][tests] Print errors found in log files

2017-11-21 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5012 LGTM. I'm puzzled why the `])` is printed out-of-order in the middle of the stack trace. ---

[GitHub] flink issue #5020: [FLINK-8084][build] Remove unnecessary japicmp pom entrie...

2017-11-21 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5020 +1 ---

[GitHub] flink pull request #5024: [hotfix][docs] Readme review to clarify heading fo...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5024#discussion_r152384448 --- Diff: docs/index.md --- @@ -23,19 +24,17 @@ specific language governing permissions and limitations under the License

[GitHub] flink pull request #5024: [hotfix][docs] Readme review to clarify heading fo...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5024#discussion_r152386126 --- Diff: docs/index.md --- @@ -23,19 +24,17 @@ specific language governing permissions and limitations under the License

[GitHub] flink pull request #5024: [hotfix][docs] Readme review to clarify heading fo...

2017-11-21 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/5024#discussion_r152384103 --- Diff: docs/README.md --- @@ -90,7 +90,7 @@ This will be replaced with the value of the variable called `NAME` when generati Headings

[GitHub] flink issue #5034: [FLINK-8105][minor] Removed unnecessary null check

2017-11-20 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5034 IntelliJ reports 24 instances of "unnecessary 'null' check before 'instanceof' expression". ---

[GitHub] flink issue #5023: [hotfix][docs] Review of concepts docs for grammar, forma...

2017-11-17 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5023 @ChrisChinchilla I think you want to `git rebase origin/master` (or whatever you have named upstream). Did you do a merge instead? ---

[GitHub] flink issue #5001: [FLINK-7679][build][java9] Upgrade maven enforcer plugin ...

2017-11-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/5001 +1 tests are passing and the 1.4 release has been forked so we're at the start of a new release cycle ---

[GitHub] flink issue #4968: [FLINK-8006] [Startup Shell Scripts] Enclosing $pid in qu...

2017-11-09 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4968 +1 ---

[GitHub] flink issue #4968: [FLINK-8006] [Startup Shell Scripts] Enclosing $pid in qu...

2017-11-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4968 @elbaulp au contraire, thank you for fixing this bug. Do we need quotes for the following? line 69: pid=$FLINK_PID_DIR/flink-$FLINK_IDENT_STRING-$DAEMON.pid line 181: done < ${pid}.

[GitHub] flink issue #4945: [FLINK-7977][build] bump version of compatibility check f...

2017-11-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4945 @zentol but we need a check against additions to the API since 1.0. Enabling `breakBuildOnModifications` in bugfix releases which don't add the API wouldn't require updating compatibility versions

[GitHub] flink issue #4965: [FLINK-8004][metrics][docs] Fix usage examples

2017-11-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4965 LGTM. ---

[GitHub] flink issue #4971: [FLINK-8010][build] Bump remaining flink-shaded versions

2017-11-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4971 Is anyone else seeing the red/green highlights shifted two lines up for this PR? I'm seeing this in multiple browsers and with plug-ins disabled but only on this PR. ---

[GitHub] flink issue #4959: [FLINK-7998] private scope is changed to public to resolv...

2017-11-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4959 Looks good but I have not tested. Can we also fix the parameter swap referenced in the JIRA? ---

[GitHub] flink issue #4968: [FLINK-8006] [Startup Shell Scripts] Enclosing $pid in qu...

2017-11-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4968 Do we also need to quote within sub-expressions? Have you looked at the stop script? ---

[GitHub] flink issue #4945: [FLINK-7977][build] bump version of compatibility check f...

2017-11-07 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4945 Isn't this API compatibility rather than checkpoint/savepoint compatibility? And if the former should not 1.4 be checked against 1.3 (which should be checked against 1.2, etc.)? ---

  1   2   3   4   5   6   7   8   9   10   >