You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by yogidevendra <gi...@git.apache.org> on 2016/05/13 06:58:20 UTC
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
GitHub user yogidevendra opened a pull request:
https://github.com/apache/incubator-apex-malhar/pull/273
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yogidevendra/incubator-apex-malhar APEXMALHAR-2077-AbstractSingleFileOutputOperator-partitionId
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-malhar/pull/273.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 #273
----
commit 1041b99cb6f69f20e288fe0a82018f8b4cc919e6
Author: yogidevendra <de...@datatorrent.com>
Date: 2016-05-13T06:03:53Z
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
----
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63443902
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
Yogi, if you can just implement the check if the format string is null or empty, we can merge this. We can defer the issue of whether dynamically changing the file name is supported for the next release. We have no guarantee on the value of the operator id (not guaranteed to start at 0).
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63228281
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
Yes I think that should be the behavior
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63226693
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
Ok. Can we at least provide a way to turn off this behavior -- e.g. if the format string is null or the empty string, the operatorId is not appended.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63195600
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
I'm wondering why we even need this class; can we not simply add a boolean property to the base class called "appendPartitionId" ?
Also, the ticket description says "If SingleFileOutputOperator is partitioned into **more than one** physical instances then it should append partitionId to file name" (my emphasis), I don't see any check for that condition before appending the partition id.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63391392
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
@yogidevendra IMO you can check if format string is empty string or null and decide whether to append partition id. A separate flag I think is redundant.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63351657
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
The **user** knows whether there are multiple partitions or just one, we don't need to figure it out. In the latter case, they need to have a way of turning off this behavior (e.g. can set the format string to null). There is no harm in doing this if it is properly documented.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63356489
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
People have different preferences on this topic; my preference is a separate flag but I'm OK either 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-apex-malhar/pull/273
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63193713
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -62,4 +92,32 @@ public String getOutputFileName()
{
return outputFileName;
}
+
+ /**
+ * @return string format specifier for the partitioned file name
+ */
+ public String getPartitionedFileNameformat()
+ {
+ return partitionedFileNameformat;
+ }
+
+ /**
+ * @param partitionedFileNameformat
+ * string format specifier for the partitioned file name
+ */
--- End diff --
The JavaDoc comment should state the expected form of the format string.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63314787
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -52,6 +81,7 @@ protected String getFileName(INPUT tuple)
public void setOutputFileName(String outputFileName)
{
this.outputFileName = outputFileName;
+ partitionedFileName = String.format(partitionedFileNameformat, outputFileName, physicalPartitionId);
--- End diff --
Let me know if this assumption is incorrect.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
GitHub user yogidevendra reopened a pull request:
https://github.com/apache/incubator-apex-malhar/pull/273
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yogidevendra/incubator-apex-malhar APEXMALHAR-2077-AbstractSingleFileOutputOperator-partitionId
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-malhar/pull/273.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 #273
----
commit 67ad24edd6dc1629501e86168d2594606c52e291
Author: yogidevendra <de...@datatorrent.com>
Date: 2016-05-13T06:03:53Z
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
6. Fixing unit tests
7. Incorporating review comments.
8. Fixing checkstyle violation
----
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra closed the pull request at:
https://github.com/apache/incubator-apex-malhar/pull/273
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63355212
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -52,6 +81,7 @@ protected String getFileName(INPUT tuple)
public void setOutputFileName(String outputFileName)
{
this.outputFileName = outputFileName;
+ partitionedFileName = String.format(partitionedFileNameformat, outputFileName, physicalPartitionId);
--- End diff --
This assumption is correct, my suggestion then is: Has this capability been tested with an actual application on a cluster (not just local mode) where the file name is changed mid-stream ? If so, suggest we document that this feature is supported and add a unit test that tests 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63321660
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
My assumption is : if we have multiple partitions then each partition must write to different file. Thus giving this choice to the end use may not be good idea.
Regarding adding operatorId suffix only in case when there are more than one partitions. This is good suggestion, but there is no direct way of getting number of physical instances for the operator in the setup method. We might need to add custom partitioner for doing this. IMO, it is over-complicating the thing for the delta value addition.
Can you think of simpler way of knowing no. of physical partitions? OR do you feel that it should be OK without worrying too much about 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63224257
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
@amberarrow AbstractSingleFileOutputOperator is not a new class. It was there 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63518655
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +43,42 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
+ {
+ super.setup(context);
+ physicalPartitionId = context.getId();
+ if(StringUtils.isEmpty(partitionedFileNameformat)){
+ partitionedFileName = outputFileName;
+ }
+ else{
--- End diff --
Good catch. I will fix this. But, just wondering why checkstyle did not catched this?
Anything 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra closed the pull request at:
https://github.com/apache/incubator-apex-malhar/pull/273
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63352546
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
OR you prefer to have explicit flag to this off? But, this flag will be honored only for single partition.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63352405
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +41,37 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
--- End diff --
OK. So, I will add check which says that if physicalPartitionId is 0 and format string is (empty or null) then do not append partition id.
Does that sounds good?
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
GitHub user yogidevendra reopened a pull request:
https://github.com/apache/incubator-apex-malhar/pull/273
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yogidevendra/incubator-apex-malhar APEXMALHAR-2077-AbstractSingleFileOutputOperator-partitionId
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-malhar/pull/273.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 #273
----
commit 67ad24edd6dc1629501e86168d2594606c52e291
Author: yogidevendra <de...@datatorrent.com>
Date: 2016-05-13T06:03:53Z
APEXMALHAR-2077 AbstractSingleFileOutputOperator appending partitionID
1. Renamed HDFSOutputOperator to BytesFileOutputOperator
2. Moved partitionID related logic into AbstractSingleFileOutputOperator
3. BytesFileOutputOperator extends AbstractSingleFileOutputOperator
4. renamed currentPartName to partitionedFileName
5. update partitionedFileName from field setters
6. Fixing unit tests
7. Incorporating review comments.
8. Fixing checkstyle violation
----
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63193475
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -62,4 +92,32 @@ public String getOutputFileName()
{
return outputFileName;
}
+
+ /**
+ * @return string format specifier for the partitioned file name
+ */
+ public String getPartitionedFileNameformat()
+ {
+ return partitionedFileNameformat;
+ }
+
+ /**
+ * @param partitionedFileNameformat
+ * string format specifier for the partitioned file name
+ */
+ public void setPartitionedFileNameformat(String partitionedFileNameformat)
+ {
+ this.partitionedFileNameformat = partitionedFileNameformat;
+ partitionedFileName = String.format(partitionedFileNameformat, outputFileName, physicalPartitionId);
--- End diff --
See note above.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by yogidevendra <gi...@git.apache.org>.
Github user yogidevendra commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63314771
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -52,6 +81,7 @@ protected String getFileName(INPUT tuple)
public void setOutputFileName(String outputFileName)
{
this.outputFileName = outputFileName;
+ partitionedFileName = String.format(partitionedFileNameformat, outputFileName, physicalPartitionId);
--- End diff --
My assumption was when we set property values for the operators in the running application using apexcli; then it will only invoke setter method and setup method will not be invoked.
Hence, I am initializing derived field values in the setter of original property.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63194215
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -52,6 +81,7 @@ protected String getFileName(INPUT tuple)
public void setOutputFileName(String outputFileName)
{
this.outputFileName = outputFileName;
+ partitionedFileName = String.format(partitionedFileNameformat, outputFileName, physicalPartitionId);
--- End diff --
Since partitionedFileName is assigned in setup() there is no need to do it here; in fact, since these setters are typically called before the operator is serialized, physicalPartitionId will always be 0 here.
---
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] incubator-apex-malhar pull request: APEXMALHAR-2077 AbstractSingle...
Posted by amberarrow <gi...@git.apache.org>.
Github user amberarrow commented on a diff in the pull request:
https://github.com/apache/incubator-apex-malhar/pull/273#discussion_r63516080
--- Diff: library/src/main/java/com/datatorrent/lib/io/fs/AbstractSingleFileOutputOperator.java ---
@@ -39,10 +43,42 @@
@NotNull
protected String outputFileName;
+ /**
+ * partitionedFileName string format specifier
+ e.g. fileName_physicalPartionId -> %s_%d
+ */
+ private String partitionedFileNameformat = "%s_%d";
+
+ /**
+ * Derived name for file based on physicalPartitionId
+ */
+ private transient String partitionedFileName;
+
+ /**
+ * Physical partition id for the current partition.
+ */
+ private transient int physicalPartitionId;
+
+ /**
+ * Initializing current partition id, partitionedFileName etc. {@inheritDoc}
+ */
+ @Override
+ public void setup(OperatorContext context)
+ {
+ super.setup(context);
+ physicalPartitionId = context.getId();
+ if(StringUtils.isEmpty(partitionedFileNameformat)){
+ partitionedFileName = outputFileName;
+ }
+ else{
--- End diff --
else on same line as the 2 braces with a space around 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---