You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by randerzander <gi...@git.apache.org> on 2016/12/07 07:43:19 UTC
[GitHub] nifi pull request #1307: NIFI-2585: Add attributes to track where a flow fil...
GitHub user randerzander opened a pull request:
https://github.com/apache/nifi/pull/1307
NIFI-2585: Add attributes to track where a flow file came from when r\u2026
### 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?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
I'm uncertain how to add a unit test for this as I'm not sure why [TestStandardSiteToSiteProtocol](https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/test/java/org/apache/nifi/remote/TestStandardSiteToSiteProtocol.java) is entirely commented out.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/randerzander/nifi master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1307.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 #1307
----
commit ccaa12b4b6e4d648c63a2e1b122a3e37b8f2b505
Author: Randy Gelhausen <rg...@gmail.com>
Date: 2016-12-07T07:18:09Z
NIFI-2585: Add attributes to track where a flow file came from when receiving over site-to-site
----
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the issue:
https://github.com/apache/nifi/pull/1307
This may be an artifact of Docker generating hostnames that don't parse well.
Here's what I see in my testing:
monitor.dev_1 | 2016-12-09 19:41:34,127 INFO [Site-to-Site Worker Thread-6] o.a.nifi.remote.SocketRemoteSiteListener Received connection from techops_web-service.dev_1.techops_dev/172.18.0.5, User DN: null
monitor.dev_1 | 2016-12-09 19:41:34,127 ERROR [Site-to-Site Worker Thread-6] o.a.nifi.remote.SocketRemoteSiteListener Handshake failed when communicating with nifi://techops_web-service.dev_1.techops_dev:42626; closing connection. Reason for failure: java.lang.IllegalStateException: Unable to get host or port from peerUrlnifi://techops_web-service.dev_1.techops_dev:42626
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the issue:
https://github.com/apache/nifi/pull/1307
The failing appveyor CI tests are deal with ListFile and TailFile issues unrelated to this PR.
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1307
@randerzander I wrote a simple Java main method with the following:
```
URI uri = new URI("nifi://techops_web-service.dev_1.techops_dev:42626");
System.out.println("Host: " + uri.getHost());
System.out.println("Port: " + uri.getPort());
```
Sure enough this prints the host as null and the port as -1. From what I can tell the "_" causes the URI to not parse properly. Since this appears to be based on the RFC for URIs, I don't think this should hold up merging this in.
I made an additional commit on my branch here https://github.com/bbende/nifi/commits/NIFI-2585 which in the case where host is null or port is -1, it will use the string "unknown" so that it looks a little nicer in the attributes.
Thoughts?
---
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 #1307: NIFI-2585: Add attributes to track where a flow fil...
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander closed the pull request at:
https://github.com/apache/nifi/pull/1307
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1307
Thanks for pointing that out, let me give it a try with minif-cpp and see if I can reproduce. I was hoping that by putting the attributes in AbstractFlowFileServerProtocol it would automatically work since SocketFlowFileServerProtocol extends that, but let me give it a try. I'll let you know what I find out.
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the issue:
https://github.com/apache/nifi/pull/1307
@bbende Thanks for the review and for the fixes! I applied your changes to my fork and I'm able to receive FlowFiles over S2S.
However, when sending from minifi-cpp, I'm getting s2s.address with a value of "null:-1", and I'm not getting any s2s.host attribute.
From the nifi logs, it's using SocketFlowFileServerProtocol - would the SocketFlowFileServerProtocol or SocketRemoteSiteListener need an update to add these properties as well?
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by randerzander <gi...@git.apache.org>.
Github user randerzander commented on the issue:
https://github.com/apache/nifi/pull/1307
@bbende, thanks for spending time looking into this.
Makes sense to me. Since you did all the work, I'll close this PR and you can open a new one with your branch.
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1307
Reviewing...
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1307
@randerzander I tested this out and I think we have to move the addition of the attributes to inside the loop where the flow files are being created. If we do it after the loop, then session.transfer has already been called on each flow file and we can no longer update it, it was causing an error when I tested it out.
I made a couple of updates and have a working version in the latest commit here:
https://github.com/bbende/nifi/commits/NIFI-2585
The changes I made were the following:
1) Move the attributes into the while loop in AbstractFlowFileServerProtocol
2) Added the same attributes in StandardRemoteGroupPort for pull-based site-to-site
3) Used "s2s." prefix for the attribute names (I know I suggest "remote." in the JIRA, but I started thinking it wasn't descriptive enough).
I tested this out with two NiFi instances on my laptop doing push and pull based s2s and it appears to be working correctly.
If you agree with the changes you could put my commit into your branch and update this PR, or if you want to close this I can submit my branch with both our commits as a new PR.
---
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 #1307: NIFI-2585: Add attributes to track where a flow file came ...
Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:
https://github.com/apache/nifi/pull/1307
@randerzander I haven't been able to reproduce this yet. Running a simple GetFile -> RPG on minifi-cpp to a regular NiFi with InputPort -> LogAttribute, I am seeing the two attributes come through correctly.
I'm trying to look at the rest of the s2s code to see how a Peer instance could be created and not get the correct host or port. Would you be able to add the following code to the end of the Peer constructor?
https://github.com/apache/nifi/blob/master/nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/Peer.java#L51
```
if (this.host == null || this.port == -1) {
throw new IllegalStateException("Unable to get host or port from peerUrl" + peerUrl);
}
```
My theory is that it somehow the parsing of the URI is considered successful, but it didn't actually find a host or port, since the Javadocs of uri.getPort() say it can return -1. Let me know if you can get that stacktrace from that added code to see what the peerUrl is when this happens. 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.
---