[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.)? ---

[GitHub] flink issue #4938: [FLINK-7968] [core] Move DataOutputSerializer and DataInp...

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

[GitHub] flink issue #4794: [build][minor] Add missing licenses

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

[GitHub] flink issue #4794: [build][minor] Add missing licenses

2017-10-30 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4794 Adding the license to `browserconfig.xml` looks fine, but why change the user configurations `masters`, `slaves`, and `zoo.cfg`? Are these even copyrightable? ---

[GitHub] flink issue #4748: [hotfix][tests] Use G1GC for tests

2017-10-03 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4748 For speed or consistency? ---

[GitHub] flink issue #4755: [hotfix] [docs] Fix broken links

2017-10-03 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4755 +1 for master and 1.3 ---

[GitHub] flink issue #4562: [FLINK-7402] Fix ineffective null check in NettyMessage#w...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4562 Merging ... ---

[GitHub] flink issue #4676: [FLINK-7625] Fix typo in metrics part of documents

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4676 Merging ... ---

[GitHub] flink issue #4575: [FLINK-7494][travis] Add license headers to '.travis.yml'...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4575 Merging ... ---

[GitHub] flink issue #4641: [hotfix][docs] Fix a typo on log name for quick start gui...

2017-09-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4641 Merging ... ---

[GitHub] flink pull request #4624: [FLINK-7410] [table] Use toString method to displa...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4624#discussion_r139187009 --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/functions/UserDefinedFunction.scala --- @@ -41,7 +41,7 @@ abstract class

[GitHub] flink pull request #3511: [Flink-5734] code generation for normalizedkey sor...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/3511#discussion_r139186699 --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java --- @@ -115,6 +115,13 @@ public Configuration

[GitHub] flink pull request #4574: [FLINK-6864] Fix confusing "invalid POJO type" mes...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4574#discussion_r139185112 --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java --- @@ -1900,7 +1900,8 @@ private boolean isValidPojoField

[GitHub] flink pull request #4574: [FLINK-6864] Fix confusing "invalid POJO type" mes...

2017-09-15 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4574#discussion_r139184023 --- Diff: docs/dev/types_serialization.md --- @@ -115,6 +115,8 @@ conditions are fulfilled: or have a public getter- and a setter- method

[GitHub] flink issue #4346: [FLINK-7199] [gelly] Graph simplification does not set pa...

2017-08-30 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4346 @NicoK I have fixed the noted issues and will merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

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

2017-08-30 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4574 I also agree that these messages have been personally useful. I think this PR is an improvement but that we could do better to note and/or emphasize the performance implications of `GenericType

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-18 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 @StephanEwen why are `null` values permitted if not in the contract of the input formats? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4529 @NicoK what do you think of creating a parent issue for your collection of network improvements? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] flink issue #4480: [FLINK-6995] [docs] Enable is_latest attribute to false

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4480 @tzulitai has this been merged? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] flink issue #4535: Eventhubs-support read from and write to Azure eventhubs

2017-08-15 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4535 @tzulitai this does sound like a good candidate for inclusion in Flink. Perhaps a separate `flink-connectors` repo would work better than Apache Bahir. --- If your project is set up for it, you

[GitHub] flink issue #4527: [FLINK-7409] [web] Make WebRuntimeMonitor reactive

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4527 @tillrohrmann please rebase now that #4492 has been merged --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] flink issue #4535: Eventhubs-support read from and write to Azure eventhubs

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4535 @zhuganghuaonnet we should also mention that new connectors are being contributed through the Flink release of [Apache Bahir](http://bahir.apache.org). --- If your project is set up for it, you

[GitHub] flink issue #4539: [FLINK-7443] [metrics] MetricFetcher store and deserializ...

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4539 +1 but I'm only seeing an improvement and not a bug --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] flink issue #4542: [FLINK-7445] [GitHub] Remove FLINK-1234 reference from PR...

2017-08-14 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4542 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-13 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 `MutableObjectIterator#next` allows/requires a `null` result: "@return The object or null if the iterator is exhausted.". The documentation could be improved, e.g. in `MergeIterator` th

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-12 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 As I understand Flink does not allow `null` records since what does it mean to partition or window a null record? `null` fields are sometimes allowed but `null` records are meaningless

[GitHub] flink issue #4525: [FLINK-7423] Always reuse an instance to get elements fro...

2017-08-11 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4525 Hi @XuPingyong thanks for submitting this PR. I'm not clear on why the `null` check was added in FLINK-4075 and `ContinuousFileProcessingTest` is not running locally for me. When calling

[GitHub] flink pull request #4472: FLINK-7368: MetricStore makes cpu spin at 100%

2017-08-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4472#discussion_r132429138 --- Diff: flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/metrics/MetricStore.java --- @@ -24,8 +24,8 @@ import org.slf4j.Logger

[GitHub] flink pull request #4346: [FLINK-7199] [gelly] Graph simplification does not...

2017-08-10 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4346#discussion_r132427211 --- Diff: flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/drivers/parameter/LongParameter.java --- @@ -52,16 +52,6 @@ public

[GitHub] flink issue #4504: [FLINK-7395] [metrics] Count bytesIn/Out without synchron...

2017-08-09 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4504 @zentol what is the root cause for this change? I see that this is marked as a blocker bug. Was this reported or discussed on the mailing list? --- If your project is set up for it, you can reply

[GitHub] flink issue #4494: [FLINK-7026] Introduce flink-shaded-asm-5

2017-08-08 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4494 +1 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

[GitHub] flink pull request #4461: [FLINK-7350] [travis] Only execute japicmp in misc...

2017-08-02 Thread greghogan
Github user greghogan commented on a diff in the pull request: https://github.com/apache/flink/pull/4461#discussion_r130957157 --- Diff: tools/travis_mvn_watchdog.sh --- @@ -145,8 +150,11 @@ esac # -nsu option forbids downloading snapshot artifacts. The only snapshot

[GitHub] flink issue #4316: [FLINK-6105] Use InterruptedIOException instead of IOExce...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4316 @zhangminglei, not sure if you saw @tedyu's comment on the Jira. There are more instances of this in the same file and package. If these are internal classes and Flink is not using

[GitHub] flink issue #4461: [FLINK-7350] [travis] Only execute japicmp in misc profil...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4461 Should we upgrade to japicmp 0.10 or delay this change until such time? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] flink issue #4460: [FLINK-7349] [travis] Only execute checkstyle in misc pro...

2017-08-02 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4460 +1; looks to be 20 to 30 minutes faster than recent test jobs on master --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] flink issue #4456: [FLINK-6996][kafka] Increase Xmx for tests

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4456 +1 this matches the parent `pom.xml`. Wondering if the same change would fix `flink-hbase` always failing for me when running `mvn verify`. --- If your project is set up for it, you can reply

[GitHub] flink issue #4346: [FLINK-7199] [gelly] Graph simplification does not set pa...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4346 @NicoK, parallelism tests have been added. There is a small refactor for `Runner` which needs more extensive rework elsewhere. --- If your project is set up for it, you can reply to this email

[GitHub] flink issue #4427: [FLINK-7304] [scripts] Simplify taskmanager GC configurat...

2017-08-01 Thread greghogan
Github user greghogan commented on the issue: https://github.com/apache/flink/pull/4427 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so

  1   2   3   4   5   6   7   8   9   10   >