[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-06 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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 fhueske
Github user fhueske commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145873239
  
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-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 StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1218#issuecomment-145486238
  
If you are cleaning up anyways, how about renaming the `RegularTask` to 
`BatchTask`? That is what it is, after all, the core task class for batch 
operations. There is also a `StreamTask`, so it would mirror that naming...


---
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 chiwanpark
Github user chiwanpark commented on a diff in the pull request:

https://github.com/apache/flink/pull/1218#discussion_r41101714
  
--- Diff: 
flink-staging/flink-tez/src/main/java/org/apache/flink/tez/runtime/TezTask.java 
---
@@ -402,7 +402,7 @@ private void initInputLocalStrategy(int inputNum) 
throws Exception {
localStub = 
initStub(userCodeFunctionType);
} catch (Exception e) {
throw new 
RuntimeException("Initializing the user code and the configuration failed" +
-   e.getMessage() 
== null ? "." : ": " + e.getMessage(), e);
+   (e.getMessage() 
== null ? "." : ": ") + e.getMessage(), e);
--- End diff --

It seems wrong parenthesis. If `e.getMessage()` is `null`, there is 
concatenation of `"."` and `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-2815] [REFACTOR] Remove Pact from class...

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

https://github.com/apache/flink/pull/1218#issuecomment-145383808
  
Looks good to merge except minor issues for me. :+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-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 chiwanpark
Github user chiwanpark commented on a diff in the pull request:

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

Although this line is commented, but there is a typo: `level="OFF"`.


---
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-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 
Date:   2015-10-01T21:58:19Z

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

commit 6403d44a9547b8a7f52f4795cd5ef4a72533906f
Author: hsaputra 
Date:   2015-10-02T18:16:59Z

Remove the word Pact from the Javadoc for ChainedDriver.

commit df2f553a7e46d18523521cfb18a402cc08ac5fce
Author: hsaputra 
Date:   2015-10-02T19:24:57Z

Use Java7 style of type resolution for new collection.

commit dbb217589178b4b7e7b059b352a867dfc4749856
Author: hsaputra 
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 
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.
---