[GitHub] flink issue #5845: [FLINK-9168][flink-connectors]Pulsar Sink connector

2018-06-14 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/5845
  
@XiaoZYang You can close this PR and create new branch to submit new PR 
since you are the creator os this one.
Did you see any error or something preventing you to close this one?


---


[GitHub] flink issue #4095: [FLINK-6877] [runtime] Activate checkstyle for runtime/se...

2017-06-30 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/4095
  
+1 for merging. The longer it waits the more conflicts it will cause


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4095: [FLINK-6877] [runtime] Activate checkstyle for runtime/se...

2017-06-10 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/4095
  
@zentol thats just matter of rebasing to feature branches. Not sure how 
would you coordinate among all feature branches


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4095: [FLINK-6877] [runtime] Activate checkstyle for runtime/se...

2017-06-09 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/4095
  
Looks like the tests not related to this PR.

+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3704: [FLINK-5756] Replace RocksDB dependency with FRocksDB

2017-04-20 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/3704
  
Yeah totally understand the reason. Just want to make sure we have plan to 
move back as soon as RocksDB usable for Flink.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #3704: [FLINK-5756] Replace RocksDB dependency with FRocksDB

2017-04-10 Thread hsaputra
Github user hsaputra commented on the issue:

https://github.com/apache/flink/pull/3704
  
I hope there is a plan to contribute back to rocksdb and we could come back 
to bring back dependencies on the fixed version. Relying on modified lib never 
good practice 


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [script] Simple fix of typo from JobManager to...

2016-05-19 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/2001#issuecomment-220454866
  
Thanks @StephanEwen , have merged the change.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [script] Simple fix of typo from JobManager to...

2016-05-18 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/2001#issuecomment-220113385
  
The failed test should not be caused by this simple change. 
Will merge unless anyone has comment by end of day.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [script] Simple fix of typo from JobManager to...

2016-05-17 Thread hsaputra
GitHub user hsaputra opened a pull request:

https://github.com/apache/flink/pull/2001

[script] Simple fix of typo from JobManager to TaskManager in 
taskmanager.sh file

Very simple fix for typo on comment in the task manager.sh file.

From `JobManager` to `TaskManager`.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hsaputra/flink simple_text_fix_taskmanager

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/2001.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 #2001


commit 861cbb47b17a329c6a2fee5aebd92d0d76abfb38
Author: Henry Saputra <hsapu...@apache.org>
Date:   2016-05-18T01:15:38Z

Simple fix of typo from JobManager to TaskManager in taskmanager.sh file.




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3768] [gelly] Clustering Coefficient

2016-05-11 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1896#issuecomment-218637517
  
Thanks for the docs update.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3768] [gelly] Clustering Coefficient

2016-05-10 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1896#discussion_r62704246
  
--- Diff: 
flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/examples/LocalClusteringCoefficient.java
 ---
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.graph.examples;
+
+import org.apache.commons.math3.random.JDKRandomGenerator;
+import org.apache.flink.api.common.JobExecutionResult;
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.api.java.ExecutionEnvironment;
+import org.apache.flink.api.java.io.CsvOutputFormat;
+import org.apache.flink.api.java.utils.DataSetUtils;
+import org.apache.flink.api.java.utils.ParameterTool;
+import org.apache.flink.graph.Graph;
+import org.apache.flink.graph.asm.translate.LongValueToIntValue;
+import org.apache.flink.graph.asm.translate.TranslateGraphIds;
+import org.apache.flink.graph.generator.RMatGraph;
+import org.apache.flink.graph.generator.random.JDKRandomGeneratorFactory;
+import org.apache.flink.graph.generator.random.RandomGenerableFactory;
+import org.apache.flink.types.IntValue;
+import org.apache.flink.types.LongValue;
+import org.apache.flink.types.NullValue;
+
+import java.text.NumberFormat;
+
+public class LocalClusteringCoefficient {
--- End diff --

I know this is just an example, but would be nice to add Javadoc 
information to give high level information why this class exists.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3768] [gelly] Clustering Coefficient

2016-05-10 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1896#issuecomment-218210466
  
Please address my comments about adding Javadoc header info to ALL new Java 
class added to source repo. Thx!


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3768] [gelly] Clustering Coefficient

2016-04-15 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1896#discussion_r59960582
  
--- Diff: 
flink-libraries/flink-gelly-examples/src/main/java/org/apache/flink/graph/examples/TriangleListing.java
 ---
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.graph.examples;
+
+import org.apache.commons.math3.random.JDKRandomGenerator;
+import org.apache.flink.api.common.JobExecutionResult;
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.api.java.ExecutionEnvironment;
+import org.apache.flink.api.java.io.CsvOutputFormat;
+import org.apache.flink.api.java.utils.DataSetUtils;
+import org.apache.flink.api.java.utils.ParameterTool;
+import org.apache.flink.graph.Graph;
+import org.apache.flink.graph.generator.RMatGraph;
+import org.apache.flink.graph.generator.random.JDKRandomGeneratorFactory;
+import org.apache.flink.graph.generator.random.RandomGenerableFactory;
+import org.apache.flink.graph.library.TriangleEnumerator;
+import org.apache.flink.graph.utils.Translate;
+import org.apache.flink.types.IntValue;
+import org.apache.flink.types.LongValue;
+import org.apache.flink.types.NullValue;
+
+import java.text.NumberFormat;
+
+public class TriangleListing {
--- End diff --

Could add JavaDoc to ALL new Java or Scala classes added to source? 
Explanation why and how it is interact with other code is something we are 
looking for. Thanks.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...

2016-04-14 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1829#issuecomment-210080951
  
I think it will go to 1.1.x release. Need some clean up to isolate the 
changes needed.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1745] Add exact k-nearest-neighbours al...

2016-04-09 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1220#issuecomment-207720551
  
Well, it seems like Travis like it =)


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1745] Add exact k-nearest-neighbours al...

2016-04-08 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1220#issuecomment-207547199
  
@danielblazevski : Sorry, but could you help rebase the conflicts for this 
PR? Thanks


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...

2016-04-03 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1829#issuecomment-205001401
  
+1 for merging to master. 

But would love to isolate the changes to exclude refactoring changes if 
possible


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...

2016-04-01 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1829#issuecomment-204614791
  
@mbalassi : There is already request from @zentol and me to clean up the 
PR, so would love to get this merge excluding the refactor parts. Thanks.

I think we should merge this to master and 1.1.0, the question is whether 
this should go to 1.0.1 release, which @fhueske and @uce think should not.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...

2016-03-25 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1829#issuecomment-201568146
  
@smarthi Could you remove some of the refactoring or clean up changes 
included with this PR?

It can be added as separate PR please. Thanks.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3657: Change access of DataSetUtils.coun...

2016-03-25 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1829#issuecomment-201567738
  
If it is on the Dataset or any extendible public APIs, I would be worried. 
The change was to expose private to public without modifying existing behavior. 
Would be ok from my side.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3529 Add template for pull requests

2016-03-22 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1729#issuecomment-199850351
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3579] Improve String concatenation

2016-03-20 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1808#issuecomment-199026680
  
Anyone merged this PR as part of commit to Flink ASF Git repo?

I wonder why asfgit close this PR



---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3529 Add template for pull requests

2016-03-02 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1729#issuecomment-191639327
  
Should be a link from CONTRIBUTING page to this one.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3385] [runtime] Fix outer join skipping...

2016-02-22 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1680#issuecomment-187253798
  
Could add description on how is the fix being done and proposed result. 
Thanks


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: corrections in the javadoc examples of tumblin...

2016-01-07 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1490#issuecomment-169706443
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: Stale Synchronous Parallel Iterations

2016-01-05 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/967#issuecomment-169143768
  
@nltran Looks like the current state of PR has conflict. Would you mind 
rebasing with latest?
Thx!


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [Refactor] [Cleanup] Change bunch of if-else c...

2016-01-05 Thread hsaputra
GitHub user hsaputra opened a pull request:

https://github.com/apache/flink/pull/1485

[Refactor] [Cleanup] Change bunch of if-else conditions with swith commands

Change bunch of if-else conditions with swith command to describe better 
readability and intent.

Switch statement shows intent to have different possible execution based on 
one value, in this case the "action" variable.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hsaputra/flink change_if_to_switch_clifrontend

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1485.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 #1485


commit 71759fc878d984577ff87138f147cf3f55f8658b
Author: Henry Saputra <hsapu...@apache.org>
Date:   2016-01-05T21:45:14Z

Change bunch of if-else conditions with swith command to describe better 
readibility and intent.




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3194] Remove standalone web client

2015-12-31 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1481#issuecomment-168232207
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3093] Introduce annotations for interfa...

2015-12-14 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1427#issuecomment-164665117
  
Since this is primarily used to tag Interface or methods, could we just 
call the annotations as Public and Experimental rather than PublicInterface and 
PublicExperimental ? 

Seemed redundant to me.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: updated to Flink v0.10.1

2015-12-09 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1443#issuecomment-163153842
  
Do you have JIRA ticket filed for this? As part of ASF contribution please 
do file JIRA [1] and associate with the PR [2]. Thanks!

[1] https://issues.apache.org/jira/browse/FLINK/
[2] https://flink.apache.org/contribute-code.html


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3147] HadoopOutputFormatBase should exp...

2015-12-08 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1442#issuecomment-163074809
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2906] Remove Record API

2015-11-25 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1403#issuecomment-159763961
  
Awesome!


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3059] Javadoc fix for DataSet.writeAsTe...

2015-11-25 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1392#issuecomment-159692006
  
Love it!

+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request:

2015-11-25 Thread hsaputra
Github user hsaputra commented on the pull request:


https://github.com/apache/flink/commit/aad99f25cd107a8eaa24a8d407680a8186ba6460#commitcomment-14623683
  
+1
This should be easy rebase to the branch.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [github] change tab size to 4 spaces

2015-11-25 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1404#issuecomment-159637645
  
Yes

+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3009] Add dockerized jekyll environment

2015-11-19 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1363#issuecomment-158245010
  
LGTM 
+1
Will commit end of day if no one beats me


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-3009] Add dockerized jekyll environment

2015-11-17 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1363#discussion_r45075237
  
--- Diff: docs/docker/run.sh ---
@@ -0,0 +1,74 @@
+#!/bin/bash
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e -x -u
+
+SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
+
+export IMAGE_NAME="flink/docs"
+
+pushd ${SCRIPT_DIR}
+
+docker build --rm=true -t ${IMAGE_NAME} .
+
+popd
+
+if [ "$(uname -s)" == "Linux" ]; then
+  USER_NAME=${SUDO_USER:=$USER}
+  USER_ID=$(id -u "${USER_NAME}")
+  GROUP_ID=$(id -g "${USER_NAME}")
+else # boot2docker uid and gid
+  USER_NAME=$USER
+  USER_ID=1000
+  GROUP_ID=50
+fi
+
+docker build -t "${IMAGE_NAME}-${USER_NAME}" - <

[GitHub] flink pull request: [FLINK-3009] Add dockerized jekyll environment

2015-11-17 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1363#issuecomment-157408961
  
Other than small nit on text, +1

Seems like Travis fails unrelated to this patch


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2441] Introduce Python OperationInfo

2015-11-16 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1352#issuecomment-157137646
  
LGTM 
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-3013: Incorrect package declaration in G...

2015-11-16 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1356#issuecomment-157136261
  
Thanks for the PR =)

There seems like some extra changes which not related to changing the 
package declaration which related to code clean up. We prefer to have code 
cleanup done as separate PR or commit.
Could you either:
1. Add more comments in the commit and PR description to add information 
about what changes included.
2. Revert the changes non related to changing the package declaration?

Thanks again for the contribution.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2914] Add missing break Statement in ZK...

2015-11-16 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1359#issuecomment-157262540
  
Any way to test for this fix?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1982] [record-api] Remove dependencies ...

2015-10-22 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1294#discussion_r42818462
  
--- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/operators/MapTaskTest.java 
---
@@ -38,8 +37,7 @@
 import org.junit.Assert;
 import org.junit.Test;
 
-@SuppressWarnings("deprecation")
-public class MapTaskTest extends 
DriverTestBase<GenericCollectorMap<Record, Record>> {
+public class MapTaskTest extends DriverTestBase<FlatMapFunction<Record, 
Record>> {
--- End diff --

Small nit: should probably rename this to ``FlatMapTaskTest`` since now it 
is actually using ``flatMap`` function instead?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2872] [Documentation] Update the docume...

2015-10-20 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1268#issuecomment-149593562
  
Will merge this sometime today if no more comments.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2872] [Documentation] Update the docume...

2015-10-19 Thread hsaputra
GitHub user hsaputra opened a pull request:

https://github.com/apache/flink/pull/1268

[FLINK-2872] [Documentation] Update the documentation for Scala part to add 
ExecutionEnvironment.readFileOfPrimitives.

Add the missing Scala part for ExecutionEnvironment.readFileOfPrimitives 
API doc in the
programming guide.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hsaputra/flink 
add_readFileOfPrimitives_scala_guide

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1268.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 #1268


commit 1209019cfca6d1890c687818792eca568920af03
Author: Henry Saputra <hsapu...@apache.org>
Date:   2015-10-19T19:06:49Z

[FLINK-2872] [Documentation] Update the documentation for Scala part to add 
readFileOfPrimitives.

Add the missing Scala part for ExecutionEnvironment.readFileOfPrimitives 
API doc in the
programming guide.




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2872] [Documentation] Update the docume...

2015-10-19 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1268#issuecomment-149372176
  
@fhueske , I have updated the PR with the Java variant info.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: Removed broken dependency to flink-spargel.

2015-10-17 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1259#issuecomment-148939135
  
@StephanEwen has this been merged as part of your other commit?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-13 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/948#issuecomment-147937031
  
Thx again for updating the patch, @ankurcha . Apologize for the delay of 
the review.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2797][cli] Add support for running jobs...

2015-10-11 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1214#issuecomment-147235260
  
Seems like each build fail in different places.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-11 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/948#discussion_r41721026
  
--- Diff: flink-mesos/pom.xml ---
@@ -0,0 +1,203 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+   
+   flink-parent
+   org.apache.flink
+   0.10-SNAPSHOT
+   ..
+   
+   4.0.0
+
+   flink-mesos
+   flink-mesos
+   jar
+
+   
+   0.23.0
+   0.8.4
+   
+
+   
+   
+   com.github.scopt
+   scopt_${scala.binary.version}
+   
+   
+   net.databinder
+   
unfiltered-jetty_${scala.binary.version}
--- End diff --

What is this dependency for? And also what is the license for it?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-11 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/948#discussion_r41721041
  
--- Diff: flink-mesos/pom.xml ---
@@ -0,0 +1,203 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+   
+   flink-parent
+   org.apache.flink
+   0.10-SNAPSHOT
+   ..
+   
+   4.0.0
+
+   flink-mesos
+   flink-mesos
+   jar
+
+   
+   0.23.0
+   0.8.4
+   
+
+   
+   
+   com.github.scopt
+   scopt_${scala.binary.version}
+   
+   
+   net.databinder
+   
unfiltered-jetty_${scala.binary.version}
+   ${unfiltererd.version}
+   
+   
+   net.databinder
+   
unfiltered-filter_${scala.binary.version}
--- End diff --

Same question:
What is this dependency for? And also what is the license for it?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2797][cli] Add support for running jobs...

2015-10-10 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1214#issuecomment-147112611
  
@sachingoel0101 , could you access this URL: 
https://travis-ci.org/apache/flink/jobs/84664370

You could always set your own Apache Flink forked repo. That way you could 
try to send PR against your own copy of the repo to kick off Travis.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-06 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145890525
  
Thanks all for the reviews. Merging ...


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-05 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145740343
  
Updated based on review.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-05 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145537573
  
Makes sense, will make that update


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-04 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1218#discussion_r41101900
  
--- Diff: flink-tests/src/test/resources/logback-test.xml ---
@@ -27,7 +27,7 @@
 
 
 
-
+
--- End diff --

Ah, good catch, will make the change.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145381091
  
Since this change involve renaming classes I will merge this end of day 
tomorrow if no more comments.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145384630
  
Hi @chiwanpark , thanks for the review. Thanks for catching the typo and 
miss parentheses.
Just push the update.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2741] - Use single log statement in Tes...

2015-10-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1221#issuecomment-145399785
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2741] - Use single log statement in Tes...

2015-10-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1221#issuecomment-14536
  
Thanks for the patch update. 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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2741] - Use single log statement in Tes...

2015-10-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1221#issuecomment-145406603
  
Thanks for second pair of eyes :)
How the output result is different? 
The patch just make sure logging of all messages happen in single log call


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2741] - Use single log statement in Tes...

2015-10-03 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1221#issuecomment-145315514
  
Unfortunately looks like this patch does not take advantage of slf4j 
parameterized logging [1].

Either do like:

log.info("=" + "\nTest " + description + " 
is running." + 
"\n");
or:

log.info("===" + "\nTest {} is running." + 
"\n",
 description);


[1] http://www.slf4j.org/faq.html#logging_performance


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2642] [table] Scala Table API crashes w...

2015-10-02 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1209#discussion_r41051842
  
--- Diff: 
flink-staging/flink-table/src/main/scala/org/apache/flink/api/table/plan/PlanTranslator.scala
 ---
@@ -110,7 +110,8 @@ abstract class PlanTranslator {
 }
 
 val clazz = repr.getType().getTypeClass
-if (clazz.isMemberClass && !Modifier.isStatic(clazz.getModifiers)) {
+if ((clazz.isMemberClass && !Modifier.isStatic(clazz.getModifiers))
+|| clazz.getCanonicalName() == null) {
--- End diff --

Gah, another Java problem of returning null =(


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2642] [table] Scala Table API crashes w...

2015-10-02 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1209#issuecomment-145115050
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...

2015-10-02 Thread hsaputra
GitHub user hsaputra opened a pull request:

https://github.com/apache/flink/pull/1218

[FLINK-2815] [REFACTOR] Remove Pact from class and file names since it is 
no longer valid reference

Remove Pact word from class and file names in Apache Flink.
Pact was the name used in Stratosphere time to refer to concept of 
distributed datasets (similar to Flink Dataset). It was used when Pact and 
Nephele still separate concept.

As part of 0.10.0 release cleanup effort, let's remove the Pact names to 
avoid confusion.

The PR also contains small cleanups (sorry):
1. Small refactor DataSinkTask and DataSourceTask to follow Java7 generic 
convention creation new collection. Remove LOG.isDebugEnabled check.
2. Simple cleanup to update MapValue and TypeInformation with Java7 generic 
convention creation new collection.
3. Combine several exceptions that have same catch operation.

Apologize for the extra changes with PR. But I separated them into 
different commits for easier review.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hsaputra/flink remove_pact_name

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1218.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 #1218


commit 0c562f4b37f448a8cb408d70aa301e6d861a464d
Author: hsaputra <hsapu...@apache.org>
Date:   2015-10-01T21:58:19Z

Small refactor on DataSinkTask and DataSourceTask classes to keep up with 
modern Java practice.

commit 6403d44a9547b8a7f52f4795cd5ef4a72533906f
Author: hsaputra <hsapu...@apache.org>
Date:   2015-10-02T18:16:59Z

Remove the word Pact from the Javadoc for ChainedDriver.

commit df2f553a7e46d18523521cfb18a402cc08ac5fce
Author: hsaputra <hsapu...@apache.org>
Date:   2015-10-02T19:24:57Z

Use Java7 style of type resolution for new collection.

commit dbb217589178b4b7e7b059b352a867dfc4749856
Author: hsaputra <hsapu...@apache.org>
Date:   2015-10-02T22:03:50Z

Simple cleanup to update MapValue with Java7 generic for new collection. 
Remove unused imports in CollectionsDataTypeTest.

commit e085f38958b62867509d7ab5997cabe858a3abaa
Author: hsaputra <hsapu...@apache.org>
Date:   2015-10-02T22:41:30Z

Remove Pact from the file names of teh flink-runtime and flink-clients 
modules.




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1599] TypeComperator with no keys and c...

2015-10-01 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1207#issuecomment-144846200
  
+1

Test fail on StormWordCountLocalITCase not related to the changes.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-01 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/948#issuecomment-144858877
  
Quick comment on the PR, could you add Javadoc header information to give 
short description on why each new class/ trait is created and what role do they 
play to support Mesos integration.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-01 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/948#discussion_r40970091
  
--- Diff: 
flink-mesos/src/main/scala/org/apache/flink/mesos/executor/FlinkExecutor.scala 
---
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.mesos.executor
+
+import org.apache.flink.configuration.{Configuration, GlobalConfiguration}
+import org.apache.flink.mesos._
+import org.apache.flink.mesos.scheduler._
+import org.apache.flink.runtime.StreamingMode
+import org.apache.mesos.Protos._
+import org.apache.mesos.{Executor, ExecutorDriver}
+
+import scala.util.{Failure, Success, Try}
+
+trait FlinkExecutor extends Executor {
--- End diff --

Could you add Javadoc header to explain why this trait is created and 
summary of when and where this would be used in the execution?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1984] Integrate Flink with Apache Mesos

2015-10-01 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/948#discussion_r40970304
  
--- Diff: 
flink-mesos/src/main/scala/org/apache/flink/mesos/executor/TaskManagerExecutor.scala
 ---
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.mesos.executor
+
+import scala.util.Try
+
+import org.apache.flink.configuration.GlobalConfiguration
+import org.apache.flink.runtime.StreamingMode
+import org.apache.flink.runtime.taskmanager.TaskManager
+import org.apache.flink.runtime.util.EnvironmentInformation
+import org.apache.mesos.{Executor, ExecutorDriver, MesosExecutorDriver}
+import org.apache.mesos.Protos.Status
+import org.slf4j.{Logger, LoggerFactory}
+
+class TaskManagerExecutor extends FlinkExecutor {
--- End diff --

Could you add Javadoc header to describe the purpose of this class?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2783] Remove "degreeOfParallelism" API ...

2015-09-30 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1200#issuecomment-144621532
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2775] [CLEANUP] Cleanup code as part of...

2015-09-29 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1189#discussion_r40695062
  
--- Diff: 
flink-staging/flink-gelly/src/main/java/org/apache/flink/graph/example/utils/ExampleUtils.java
 ---
@@ -117,7 +117,7 @@ public void configure(Configuration parameters) {
new FlatMapFunction<Long, Edge<Long, 
NullValue>>() {
@Override
public void flatMap(Long key,
-   Collector<Edge<Long, 
NullValue>> out)
+   
Collector<Edge<Long, NullValue>> out)
--- End diff --

Thank! Will update it.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2775] [CLEANUP] Cleanup code as part of...

2015-09-29 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1189#issuecomment-144114134
  
Thanks guys,  updated based comments. Will merge end of day if no more 
comments.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2775] [CLEANUP] Cleanup code as part of...

2015-09-29 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1189#issuecomment-144136876
  
Build passes, will squash commits and 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [CLEANUP] Cleanup code as part of theme to be ...

2015-09-28 Thread hsaputra
GitHub user hsaputra opened a pull request:

https://github.com/apache/flink/pull/1189

[CLEANUP] Cleanup code as part of theme to be more consistent

As part of the theme to help make the code more consistent, add cleanup to 
Utils classes:
-) Add final class modifier to the XXXUtils and XXXUtil classes to make 
sure can not be extended.
-) Add missing Javadoc header classs to some public classes.
-) Add private constructor to Utils classes to avoid instantiation.
-) Remove unused 
test/java/org/apache/flink/test/recordJobs/util/ConfigUtils.java class

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/hsaputra/flink add_missing_javadocs_class

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/1189.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 #1189


commit eeb10e6eb43441c445c920c5af2dfe3f55c8ba74
Author: hsaputra <hsapu...@apache.org>
Date:   2015-09-28T19:56:16Z

Cleanup code as part of theme to be more consistent:
-) Add final class modifier to the XXXUtils and XXXUtil classes to make 
sure could not be extended.
-) Add missing Javadoc header classs to some public classes.
-) Add private constructor to Utils classes to avoid instantiation.
-) Remove unused 
test/java/org/apache/flink/test/recordJobs/util/ConfigUtils.java class




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [Flink-2768][Documentation] Fix wrong java ver...

2015-09-28 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1188#issuecomment-143862604
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [Flink-2768][Documentation] Fix wrong java ver...

2015-09-28 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1188#issuecomment-143862641
  
Could you squash the commits into one please?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2775] [CLEANUP] Cleanup code as part of...

2015-09-28 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1189#issuecomment-143916305
  
Looks like fail on unrelated Kafka partition error 


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...

2015-09-23 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1174#issuecomment-142667072
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...

2015-09-23 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1174#issuecomment-142707617
  
But we do shade netty in the flink-shaded-hadoop module [1], don't we?
By explicit pulling specific Netty version I think it would just mean it 
will shade that particular version of Netty instead of the one from Hadoop. Or 
maybe I am wrong.

[1] 
https://cwiki.apache.org/confluence/display/FLINK/Hadoop+Versions+and+Dependency+Shading


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2651] Add Netty to dependency managemen...

2015-09-23 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1174#issuecomment-142755444
  
That is good to know because I did similar thing in other project to make 
sure we have the right Netty version. But no shading involve either.

Agree about Guava for sure =(




---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2656] Fix behavior of FlinkKafkaConsume...

2015-09-10 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1117#discussion_r39193575
  
--- Diff: 
flink-staging/flink-streaming/flink-streaming-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/LegacyFetcher.java
 ---
@@ -356,29 +357,24 @@ public void run() {
// make sure that all partitions have some 
offsets to start with
// those partitions that do not have an offset 
from a checkpoint need to get
// their start offset from ZooKeeper
-   
-   List partitionsToGetOffsetsFor 
= new ArrayList<>();
+   {
--- End diff --

Just curious what is the extra "{" for?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2656] Fix behavior of FlinkKafkaConsume...

2015-09-10 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1117#issuecomment-139331689
  
Hi @rmetzger, could you tell bit summary what the fix should do in the 
description of this PR? Thanks.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-08 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1093#discussion_r38966751
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/core/memory/MemorySegmentFactory.java 
---
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.core.memory;
+
+import java.nio.ByteBuffer;
+
+/**
+ * A factory for memory segments. The purpose of this factory is to make 
sure that all memory segments
+ * for heap data are of the same type. That way, the runtime does not mix 
the various specializations
+ * of the {@link org.apache.flink.core.memory.MemorySegment}. Not mixing 
them has shown to be beneficial
+ * to method specialization by the JIT and to overall performance.
+ * 
+ * Note that this factory auto-initialized to use {@link 
org.apache.flink.core.memory.HeapMemorySegment},
+ * if a request to create a segment comes before the initialization.
+ */
+public class MemorySegmentFactory {
+
+   /** The factory to use */
+   private static Factory factory;
--- End diff --

AH, looks like you already addressed it in the actual commit but not 
reflected in the PR =)

Echoing others comment this is an awesome PR, @StephanEwen !


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-08 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1093#issuecomment-138676857
  
@StephanEwen ah yes I saw the update =)
Sorry I missed it in the commit check, thanks again for the hard work!


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-08 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1093#discussion_r38966473
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/core/memory/MemorySegmentFactory.java 
---
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.core.memory;
+
+import java.nio.ByteBuffer;
+
+/**
+ * A factory for memory segments. The purpose of this factory is to make 
sure that all memory segments
+ * for heap data are of the same type. That way, the runtime does not mix 
the various specializations
+ * of the {@link org.apache.flink.core.memory.MemorySegment}. Not mixing 
them has shown to be beneficial
+ * to method specialization by the JIT and to overall performance.
+ * 
+ * Note that this factory auto-initialized to use {@link 
org.apache.flink.core.memory.HeapMemorySegment},
+ * if a request to create a segment comes before the initialization.
+ */
+public class MemorySegmentFactory {
+
+   /** The factory to use */
+   private static Factory factory;
--- End diff --

@StephanEwen since you already merge this PR, could file JIRA to follow up 
on this one? Want to make sure we got this right.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-08 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1093#discussion_r38951685
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/core/memory/MemorySegmentFactory.java 
---
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.core.memory;
+
+import java.nio.ByteBuffer;
+
+/**
+ * A factory for memory segments. The purpose of this factory is to make 
sure that all memory segments
+ * for heap data are of the same type. That way, the runtime does not mix 
the various specializations
+ * of the {@link org.apache.flink.core.memory.MemorySegment}. Not mixing 
them has shown to be beneficial
+ * to method specialization by the JIT and to overall performance.
+ * 
+ * Note that this factory auto-initialized to use {@link 
org.apache.flink.core.memory.HeapMemorySegment},
+ * if a request to create a segment comes before the initialization.
+ */
+public class MemorySegmentFactory {
+
+   /** The factory to use */
+   private static Factory factory;
--- End diff --

+1 for that. Want to make sure we do concurrency in the right way =)


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-08 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1093#issuecomment-138636557
  
I agree to keep on-heap memory management as default. Will need to test on 
YARN deployment to make sure the kinks are resolved when working with it.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-07 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1093#discussion_r38890190
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/core/memory/MemorySegmentFactory.java 
---
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.core.memory;
+
+import java.nio.ByteBuffer;
+
+/**
+ * A factory for memory segments. The purpose of this factory is to make 
sure that all memory segments
+ * for heap data are of the same type. That way, the runtime does not mix 
the various specializations
+ * of the {@link org.apache.flink.core.memory.MemorySegment}. Not mixing 
them has shown to be beneficial
+ * to method specialization by the JIT and to overall performance.
+ * 
+ * Note that this factory auto-initialized to use {@link 
org.apache.flink.core.memory.HeapMemorySegment},
+ * if a request to create a segment comes before the initialization.
+ */
+public class MemorySegmentFactory {
+
+   /** The factory to use */
+   private static Factory factory;
--- End diff --

Should we add volatile modifier to make sure all threads looking at the 
same copy?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: FLINK-2605 Unclosed RandomAccessFile may leak ...

2015-09-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1089#issuecomment-137857005
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-1320] [core] Add an off-heap variant of...

2015-09-04 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1093#issuecomment-137857146
  
Isn't there another PR sent for this issue by Max? Or was it for something 
else.


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2326] Write yarn properties file to tem...

2015-08-27 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1062#issuecomment-135518748
  
Confirming that with my local travis also work. +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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2326] Write yarn properties file to tem...

2015-08-26 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1062#issuecomment-135177677
  
+1 LGTM

The build errors seem unrelated, but @rmetzger could you try in your fork 
to see if they pass?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2386] Rework Kafka consumer for Flink

2015-08-21 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1028#issuecomment-133463879
  
Since it is rebased to #1039 already, could this one be closed and do 
review on that one instead?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2386] Rework Kafka consumer for Flink

2015-08-21 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1028#issuecomment-133463429
  
So is the #1039 depends on this one?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2386] Add new KafkaConsumer, based on K...

2015-08-21 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1039#discussion_r37681201
  
--- Diff: 
flink-contrib/flink-storm-compatibility/flink-storm-compatibility-core/src/main/java/org/apache/flink/stormcompatibility/api/FlinkLocalCluster.java
 ---
@@ -41,8 +41,7 @@ public void submitTopology(final String topologyName, 
final Map?, ? conf, fina
 
public void submitTopologyWithOpts(final String topologyName, final 
Map?, ? conf, final FlinkTopology topology,
final SubmitOptions submitOpts) throws Exception {
-   ClusterUtil
-   
.startOnMiniCluster(topology.getStreamGraph().getJobGraph(topologyName), 
topology.getNumberOfTasks());
+   
ClusterUtil.startOnMiniCluster(topology.getStreamGraph().getJobGraph(topologyName),
 topology.getNumberOfTasks(), -1);
--- End diff --

Why is this changed? I think by default will also set it to -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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2386] Add new KafkaConsumer, based on K...

2015-08-21 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1039#discussion_r37681294
  
--- Diff: flink-core/src/main/java/org/apache/flink/util/NetUtils.java ---
@@ -48,8 +48,9 @@ public static String getHostnameFromFQDN(String fqdn) {
 * Works also for ipv6.
 *
 * See: 
http://stackoverflow.com/questions/2345063/java-common-way-to-validate-and-convert-hostport-to-inetsocketaddress
+* @return URL object for accessing host and Port
 */
-   public static void ensureCorrectHostnamePort(String hostPort) {
+   public static URL ensureCorrectHostnamePort(String hostPort) {
--- End diff --

Since it is now returning something, could we change the name to reflect it 
properly, like ``generateCorrectUrlFromHostnameAndPort``


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [CLEANUP] Add space between quotes and plus si...

2015-08-17 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1010#issuecomment-132005060
  
Thanks all, will do


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2458][FLINK-2449]Access distributed cac...

2015-08-16 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/970#discussion_r37143896
  
--- Diff: 
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
 ---
@@ -897,7 +897,7 @@ class TaskManager(
 config.timeout,
 libCache,
 fileCache,
-runtimeInfo)
+new TaskRuntimeInfo(hostname, taskManagerConfig, 
tdd.getAttemptNumber))
--- End diff --

Why is this changed from before?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: some small changes for easier understanding.

2015-08-16 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1018#issuecomment-131583486
  
+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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2512]Add client.close() before throw Ru...

2015-08-14 Thread hsaputra
Github user hsaputra commented on the pull request:

https://github.com/apache/flink/pull/1009#issuecomment-131148157
  
All tests are failing though, not sure if bc this patch


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2512]Add client.close() before throw Ru...

2015-08-14 Thread hsaputra
Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/flink/pull/1009#discussion_r37087438
  
--- Diff: 
flink-contrib/flink-storm-compatibility/flink-storm-compatibility-core/src/main/java/org/apache/flink/stormcompatibility/api/FlinkSubmitter.java
 ---
@@ -99,23 +99,24 @@ public static void submitTopology(final String name, 
final Map stormConf, final
 
final String serConf = JSONValue.toJSONString(stormConf);
 
-   final FlinkClient client = 
FlinkClient.getConfiguredClient(stormConf);
-   if (client.getTopologyJobId(name) != null) {
-   throw new RuntimeException(Topology with name ` + 
name + ` already exists on cluster);
-   }
-   String localJar = System.getProperty(storm.jar);
-   if (localJar == null) {
-   try {
-   for (final File file : ((ContextEnvironment) 
ExecutionEnvironment.getExecutionEnvironment())
-   .getJars()) {
-   // TODO verify that there is onnly one 
jar
-   localJar = file.getAbsolutePath();
+   try {
+   final FlinkClient client = 
FlinkClient.getConfiguredClient(stormConf);
--- End diff --

Shouldn't declaration of ``client`` be outside of the try clause?


---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


  1   2   3   >