You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by hsaputra <gi...@git.apache.org> on 2015/10/03 05:55:46 UTC
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
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 <hs...@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 <hs...@apache.org>
Date: 2015-10-02T18:16:59Z
Remove the word Pact from the Javadoc for ChainedDriver.
commit df2f553a7e46d18523521cfb18a402cc08ac5fce
Author: hsaputra <hs...@apache.org>
Date: 2015-10-02T19:24:57Z
Use Java7 style of type resolution for new collection.
commit dbb217589178b4b7e7b059b352a867dfc4749856
Author: hsaputra <hs...@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 <hs...@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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by chiwanpark <gi...@git.apache.org>.
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 @@
<appender-ref ref="STDOUT"/>
</root>
- <!--<logger name="org.apache.flink.runtime.operators.RegularPactTask" level="OFF"/>-->
+ <!--<logger name="org.apache.flink.runtime.operators.RegularTaskvel="OFF"/>-->
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by chiwanpark <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/flink/pull/1218#discussion_r41101884
--- 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 --
You are right, mea culpa from 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by StephanEwen <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 @@
<appender-ref ref="STDOUT"/>
</root>
- <!--<logger name="org.apache.flink.runtime.operators.RegularPactTask" level="OFF"/>-->
+ <!--<logger name="org.apache.flink.runtime.operators.RegularTaskvel="OFF"/>-->
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by hsaputra <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by chiwanpark <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2815] [REFACTOR] Remove Pact from class...
Posted by fhueske <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---