You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by apsaltis <gi...@git.apache.org> on 2016/08/23 03:24:49 UTC
[GitHub] nifi pull request #914: Adding support for Binary Row Keys
GitHub user apsaltis opened a pull request:
https://github.com/apache/nifi/pull/914
Adding support for Binary Row Keys
Adding support for Binary Row Keys for both PutHBaseCell and PutHBaseJSON. This also involved making changes to PutFlowFile and PutColumn to carry around byte[] and not all strings. These changes are per JIRA NIFI-2620
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apsaltis/nifi nifi-2620
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/914.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 #914
----
commit 34659ab69a4bfa6ca0e21f10d158c5d07f44cf26
Author: Andrew Psaltis <ps...@gmail.com>
Date: 2016-08-23T02:34:45Z
Adding support for Binary Row Keys for both PutHBaseCell and PutHBaseJSON. This also involved making changes to PutFlowFile and PutColumn to carry around byte[] and not all strings. These changes are per JIRA NIFI-2620
----
---
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] nifi pull request #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/914#discussion_r75859429
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/test/java/org/apache/nifi/hbase/MockHBaseClientService.java ---
@@ -131,4 +131,48 @@ public void setThrowException(boolean throwException) {
public byte[] toBytes(final String s) {
return s.getBytes(StandardCharsets.UTF_8);
}
+
+ @Override
+ //Implementation of this and the isHexDigit and toBinaryFromHex are from HBase Bytes.java
+ public byte[] toBytesBinary(String s) {
--- End diff --
I'm wondering if we should declare a test dependency on HBase 1.1.2 so that we don't need to copy this code, or if its better/easier to just copy this code we might need to put something in our notice or license to attribute 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] nifi pull request #914: Adding support for Binary Row Keys
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/914
---
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] nifi issue #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/914
Tested and all looks good, will merge to master, thanks Andrew!
---
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] nifi pull request #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/914#discussion_r75858615
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/PutHBaseCell.java ---
@@ -82,8 +84,19 @@ public void process(final InputStream in) throws IOException {
}
});
- final Collection<PutColumn> columns = Collections.singletonList(new PutColumn(columnFamily, columnQualifier, buffer));
- return new PutFlowFile(tableName, row, columns, flowFile);
+
+ final Collection<PutColumn> columns = Collections.singletonList(new PutColumn(columnFamily.getBytes(StandardCharsets.UTF_8),
+ columnQualifier.getBytes(StandardCharsets.UTF_8), buffer));
+
+ //check to see if we need to modify the rowKey before we pass it down to the PutFlowFile
+ byte[] rowKeyBytes = null;
--- End diff --
Might be able to create a utility method in the abstract class like `byte[] getRow(String row, String encoding)` since it looks PutHBaseCell and PutHBaseJson both need the same logic
---
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] nifi issue #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/914
Thanks for making the updates, looks good, testing it now... I noticed this PR is against 0.x, since most new development is going into 1.x, should we put this into master?
I think the changes should apply cleanly against master so we don't have to close and re-open the PR, but just wanted to confirm where it will get merged in. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/914
Overall this looks good, thanks Andrew!
One overall comment is that we now have several places where we create a new string from a byte[] like `new String(byte[])` and I think we want to specify the charset in those cases
---
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] nifi pull request #914: Adding support for Binary Row Keys
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/914#discussion_r75858286
--- Diff: nifi-nar-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.java ---
@@ -170,7 +193,7 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
}
protected String getTransitUri(PutFlowFile putFlowFile) {
- return "hbase://" + putFlowFile.getTableName() + "/" + putFlowFile.getRow();
+ return "hbase://" + putFlowFile.getTableName() + "/" + new String(putFlowFile.getRow());
--- End diff --
we may want to do `new String(putFlowFile.getRow(), StandardCharsets.UTF8)` just to be safe
---
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] nifi issue #914: Adding support for Binary Row Keys
Posted by a-aravindan <gi...@git.apache.org>.
Github user a-aravindan commented on the issue:
https://github.com/apache/nifi/pull/914
Hi All,
This nifi-**-nar-0.6.1.nar created for this request is not working with HDP 2.3.4. It is throwing the ERROR "failed to process due to java.lang.NoSuchMethodError: org.apache.nifi.hbase.put.PutColumn."
nar files used:
nifi-standard-services-api-nar-0.6.1.nar
nifi-hbase-nar-0.6.1.nar
nifi-hbase_1_1_2-client-service-nar-0.6.1.nar
Thanks
Aravind
---
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] nifi issue #914: Adding support for Binary Row Keys
Posted by apsaltis <gi...@git.apache.org>.
Github user apsaltis commented on the issue:
https://github.com/apache/nifi/pull/914
Bryan,
Thanks for the review. This is certainly new development and thus I agree that this should be put into master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your 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.
---