You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by markap14 <gi...@git.apache.org> on 2016/07/20 17:13:58 UTC

[GitHub] nifi pull request #690: NIFI-2035: Verify existence of source and destinatio...

GitHub user markap14 opened a pull request:

    https://github.com/apache/nifi/pull/690

    NIFI-2035: Verify existence of source and destination when creating a\u2026

    \u2026 connection

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/markap14/nifi NIFI-2035

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/690.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 #690
    
----
commit 07cc1748864a69ded3a62e6cb0debe22e4183586
Author: Mark Payne <ma...@hotmail.com>
Date:   2016-06-20T18:55:08Z

    NIFI-2035: Verify existence of source and destination when creating a connection

----


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @YolandaMDavis I updated the PR with a new commit so that you can see what changed. It looks like the way to cause it to hit is to drag a connection from a Processor to the Remote Process Group, and then once the dialog is up to choose which port, remove the port from a different browser tab, and then wait 30 seconds for the RPG to refresh ports. Then click the "Apply" button on the "Add Connection" dialog


---
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 #690: NIFI-2035: Verify existence of source and destinatio...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/690


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @markap14 thanks for the update! will retest and confirm.


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @YolandaMDavis I will look into the NPE. The test case that you described will not cause the issue that you are expecting, though. There is no way to test it without doing some funky stuff with breakpoints, etc. It is a timing issue. Would recommend that a code review and testing that the change doesn't break anything is sufficient.


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @markap14 during testing I am seeing unexpected behaviors/errors and wanted to validate my test case. My configuration is as follows:
    
    - 1 standalone nifi server with a Processor Group and an output port
    - A 2 node cluster with a Processor Group containing an input port and a Remote Processor Group that points to the standalone server
    
    The goal of the test was to ensure that if an attempt is made to create a new connection from the Remote Processor Group to the Processor Group on the cluster, and the output port on the standalone (remote pg) is removed just before the add, then the system will display/log IllegalArgumentException (specifically related to the source not found).
    
    During my tests the behavior has been mostly the following:
    
    1) A NullPointerException occurs just before the verifyCreate method in StandardConnectionDAO is called
    2) If the port is removed a few seconds before creating a connection then the connection is created successfully with no logged errors.
    
    Both errors prevented me from testing that IllegalArgumentExceptions would be thrown in this case (since the nullpointer exception happened 
    
    Attached are snapshots of the nifi-user.log with the nullpointer exception that occurred on several of the runs.
    
    Is this test a feasible one and appropriate for this case? If so I would recommend resolution of this issue before restesting.
    [node2-nifi-user-snapshot.log.txt](https://github.com/apache/nifi/files/395231/node2-nifi-user-snapshot.log.txt)
    <img width="1440" alt="unexpectedconnectionsuccess" src="https://cloud.githubusercontent.com/assets/1371858/17317043/8df4bc98-5847-11e6-8d1e-39fc4703bd29.png">
    Also have video of errors if needed
    
    
    
    
    



---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @markap14 retried tests again using a similar setup as before yet included an input port on the standalone server in addition to the output port.  Performed a variety of tests attempting to create and update connections made to and from the Remote Processor Group. I did see (after deleting ports and waiting 30 seconds) that an appropriate error message was received based on the null checks you made on your last commit. And when I attempted to delete and create/update a connection quickly I saw the system eventually display a warning bulletin that the connection was invalid.
    
    I was unable to trigger any of the IllegalArgumentExceptions that were added to verifyCreate in the StandardConnectionDAO, which made me wonder whether or not recent changes prevented that from even occurring? Or whether that it's a very small edge case being accounted for?
    



---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @markap14 thanks it looks good to me and no you didn't break anything :). 
    
    +1
    
    I'll get this merged in shortly.


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @markap14 picked up for 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] nifi issue #690: NIFI-2035: Verify existence of source and destination when ...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    @YolandaMDavis after reviewing the NPE I changed my mind - you do appear to be causing the issue manually :) The NPE was another place where we needed to verify that the source and destination existed. However, you will sometimes see that it creates the connection successfully, which is why I didn't think your method would work to test. The RPG refreshes the ports in the background, so depending on when you try to create the connection, it may or may not believe the port exists. Of course, a NullPointer is always wrong, though. Will update PR to address that.


---
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 #690: NIFI-2035: Verify existence of source and destination when ...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/690
  
    Hey @YolandaMDavis - the changes that were made in the later commit will not completely prevent the IllegalArgumentException from being thrown in verifyCreate... it's still possible because after the authorization is performed, it's still possible for the background thread to change the ports. But you'd have to have *REALLY* good timing to hit that :) Very unlikely you'd be able to trigger it intentionally. So if you think the change looks reasonable and I didn't break anything, I think it should be good to go


---
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.
---