You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by tasanuma <gi...@git.apache.org> on 2017/09/11 06:37:07 UTC
[GitHub] nifi pull request #2143: Nifi 4338
GitHub user tasanuma opened a pull request:
https://github.com/apache/nifi/pull/2143
Nifi 4338
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
- [x] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tasanuma/nifi NIFI-4338
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2143.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 #2143
----
commit d5224804c4701f4f0646087a977e2f08aab906e1
Author: Takanobu Asanuma <ta...@yahoo-corp.jp>
Date: 2017-09-08T08:56:28Z
add SSL Context Service to HDFS processors
commit ca8ecaa84583a83af486479c3cc896d8ee482dc0
Author: Takanobu Asanuma <ta...@yahoo-corp.jp>
Date: 2017-09-11T06:18:22Z
add displayName
commit 5eed6bb70cb76548fbd2f56994a94a7e864e7feb
Author: Takanobu Asanuma <ta...@yahoo-corp.jp>
Date: 2017-09-11T06:26:27Z
Merge branch 'master' into NIFI-4338
----
---
[GitHub] nifi issue #2143: NiFi-4338: Add documents for how to use SSL protocol from ...
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
@ijokarumawak Thanks for the review and the comments! Yes, all the additionalDetails.html is the same. I agree with you that putting it in the document of PutHDFS. I updated my pull request based on your advice. Since I don't want Parquet processors depend on HDFS processors, I also added PutParquet/additionalDetails.html.
---
[GitHub] nifi issue #2143: Nifi-4338: Support SSL Context Service in HDFS processors
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
Hello Bryan, thanks for the review!
I see. Other processors which use SSL Context Service use SSLContext class to pass the SSL/TLS values. I will investigate whether hadoop client can use it or not.
---
[GitHub] nifi pull request #2143: NiFi-4338: Add documents for how to use SSL protoco...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2143
---
[GitHub] nifi pull request #2143: NiFi-4338: Add documents for how to use SSL protoco...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2143#discussion_r143318740
--- Diff: nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/CreateHadoopSequenceFile.java ---
@@ -64,7 +64,8 @@
@SideEffectFree
@InputRequirement(Requirement.INPUT_REQUIRED)
@Tags({"hadoop", "sequence file", "create", "sequencefile"})
-@CapabilityDescription("Creates Hadoop Sequence Files from incoming flow files")
+@CapabilityDescription("Creates Hadoop Sequence Files from incoming flow files."
+ + " If you want to use SSL-secured file system like swebhdfs, please see the 'SSL Configuration' topic of the 'Additional Details' of PutHDFS.")
--- End diff --
@joewitt I agree, the explanation is not a 'capability description' rather a documentation for configuration detail.
Is there any better place to write a common topic like this for components within a NAR package? Something like 'package.html' in Javadoc, describing what the NAR includes and common configuration, what type of flow user can construct with provided processors ... etc. Is there any improvement going on in 'Extension Registry' sub project for such documentation? If not, does it sounds helpful? If so, I will raise a JIRA to add 'bundle.html' or 'nar.html' (Probably with Markdown support). How do you think?
---
[GitHub] nifi issue #2143: NiFi-4338: Add documents for how to use SSL protocol from ...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/2143
thanks @tasanuma . I agree. Am reviewing now and if checks out will merge shortly.
---
[GitHub] nifi issue #2143: Nifi-4338: Support SSL Context Service in HDFS processors
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
I reverted the last commit and added some documents for how to use SSL protocol as 'additionalDetails.html' in HDFS processors. Please review it when you are free.
---
[GitHub] nifi issue #2143: Nifi-4338: Support SSL Context Service in HDFS processors
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/2143
Hello, thank you for contributing!
Is there a way to pass the SSL/TLS values to Hadoop client without setting system properties?
Setting system properties will impact everything running in the JVM and could cause other running processors/libraries to have issues.
---
[GitHub] nifi issue #2143: NiFi-4338: Add documents for how to use SSL protocol from ...
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
Thanks for reviewing and committing it, @joewitt and @ijokarumawak!
---
[GitHub] nifi issue #2143: NiFi-4338: Add documents for how to use SSL protocol from ...
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
Thanks for the review and the comments, @joewitt and @ijokarumawak. I understand. I simply added the description to the option in the parent processor of HDFS.
I created a common additionalDetails.html in PutHDFS for now, but later it would be useful if there is a common space which is refered from processors.
---
[GitHub] nifi issue #2143: Nifi-4338: Support SSL Context Service in HDFS processors
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
I found that Hadoop has some properties for SSL protocols.
https://hadoop.apache.org/docs/r2.8.0/hadoop-mapreduce-client/hadoop-mapreduce-client-core/EncryptedShuffle.html
After setting the properties, I confirmed that SWebHdfs can be used from HDFS processors.
I would like to add the instruction to the documents of HDFS processors in this pull request.
Thanks!
---
[GitHub] nifi issue #2143: Nifi-4338: Support SSL Context Service in HDFS processors
Posted by tasanuma <gi...@git.apache.org>.
Github user tasanuma commented on the issue:
https://github.com/apache/nifi/pull/2143
I did some system tests for this change and checked it works fine. But it is hard to write a unit test for using SWebHdfs from NiFi and I was not able to do.
This is my first contribution to NiFi. Please let me know if there is any problem.
---
[GitHub] nifi issue #2143: NiFi-4338: Add documents for how to use SSL protocol from ...
Posted by ijokarumawak <gi...@git.apache.org>.
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/2143
@tasanuma Thanks for writing the detailed documentations, those are very helpful! Are the contents of each additionalDetails.html the same among those processors?
If so, we can consolidate it to a single additionalDetails.html, such as PutHDFS (or whichever), then add a short description about swebhdfs in other XXXHDFS processors '@CapabilityDescription', like "To use swebhdfs, see PutHDFS 'additional details' documentation.". Together with '@SeeAlso', user can reach to the documentation when needed. It would provide better maintainability. How do you think?
---
[GitHub] nifi pull request #2143: NiFi-4338: Add documents for how to use SSL protoco...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2143#discussion_r143278990
--- Diff: nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/CreateHadoopSequenceFile.java ---
@@ -64,7 +64,8 @@
@SideEffectFree
@InputRequirement(Requirement.INPUT_REQUIRED)
@Tags({"hadoop", "sequence file", "create", "sequencefile"})
-@CapabilityDescription("Creates Hadoop Sequence Files from incoming flow files")
+@CapabilityDescription("Creates Hadoop Sequence Files from incoming flow files."
+ + " If you want to use SSL-secured file system like swebhdfs, please see the 'SSL Configuration' topic of the 'Additional Details' of PutHDFS.")
--- End diff --
I think we should avoid adding the 'if you want to use SSL-Secured file system" entry in all these descriptions. If it is an option of the processor then on that option this comment can exist.
---