[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
GitHub user tequalsme opened a pull request:

    https://github.com/apache/incubator-nifi/pull/53

    NIFI-570: Added MongoDB put and get processors

   

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

    $ git pull https://github.com/tequalsme/incubator-nifi NIFI-570

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

    https://github.com/apache/incubator-nifi/pull/53.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 #53
   
----
commit 8c0a8f67860a1a65e583f26dc122a82aa4c773c1
Author: Tim Reardon <[hidden email]>
Date:   2015-05-05T18:51:45Z

    NIFI-570: Added MongoDB put and get processors

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
Github user joewitt commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100324870
 
    Tim - this is really awesome for a couple reasons.  First, adding support for MongoDB is cool.  Second, I've only done the eyeball test but this looks great, well thought through, well written.  It is great that you even updated the NOTICE.  I believe though the copyright mention should be 'Copyright (C) 2008-2013 10gen, Inc.' as that is what they had in their LICENSE addendum section for the version you've chosen.
   
    Is there any reason not to go straight to 3.0.x driver series?
   
    Thanks!
    Joe


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user tequalsme commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100329244
 
    Good eye, Joe. I was about to comment on a couple of things for this pull request:
   
    1 - I wasn't sure what exactly the verbiage should be in the NOTICE. I guessed based on their website, and it looks like I guessed wrong. I'll get it from their LICENSE and update.
    2 - I am using embedmongo-maven-plugin for my unit tests. I want to make sure it's ok to introduce this new dependency, could someone validate? https://github.com/joelittlejohn/embedmongo-maven-plugin
    3 - The 3.0 Java driver series was released a month ago and has a brand new API (that I have not used). The 2.x API is still supported but marked deprecated. I though it best to go with 2.x than to introduce a slew of deprecation warnings. But, since these processors are so simple, maybe it would be best for me to use the new API.
   
    Thanks for 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user joewitt commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100330127
 
    Tim
   
    1) You did a great job.  That you did it at all is highly appreciated.
    2) This is fine.  Because it is a test dependency and we're not pulling in source nor would it be part of a convenience binary we'd put out the burden is extremely small.  It is also apache licensed so woohoo.  Now it could pull in deps that aren't legit.  I didn't check.  But it doesn't matter given my previous statements (for this case).
    3) I am not a mongo expert at all.  Perhaps this is worth tossing out as a question to the dev mailing list and see if anyone has an opinion on it you can discuss with.
   
    We will do a more thorough review as well and send feedback.
   
    But clearly a quality effort so thank you very much.
    Joe


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user brianghig commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100330398
 
    +1 for the 3.0 drivers.  APIs are different, but not wildly.  Exception / error handling seemed to be the biggest change (moving to generic MongoCommandExceptions with error codes and messages).
   
    I've also done some work to support batch inserts of FlowFiles that we can merge into this to get some great throughput enhancements.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user tequalsme commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100330473
 
    No problem, however I think I made a last minute change that broke a test. I'll put up a new pull request.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user tequalsme commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100331758
 
    @brianghig will do on the 3.0 driver.
   
    +1 on the batching. I thought it best to keep it simple initially.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user brianghig commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100332336
 
    @tequalsme Absolutely! This is a great start. I'm excited that's it here now, and looking forward to seeing how far we can take 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user tequalsme commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-100997969
 
    OK, I've updated the PR to use version 3.0.1 of the MongoDB Java driver.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user bjames301 commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-102118020
 
    This is really cool Tim! Outstanding work!


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user bjames301 commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-102119394
 
    Tim, would it be helpful if I did some performance benchmarks using this processor? Writes to MongoDB tend to be very fast. This could help developers make informative choices on which processors they chose to use.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user markap14 commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-102217875
 
    Tim,
   
    Only thing that I would change about the code is to make sure we do a Provenance SEND event for PutMongo. Otherwise, all looks great!


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user tequalsme commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-103469244
 
    Updated PR, adding a Provenance SEND event to PutMongo, and rebasing origin/develop.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-nifi/pull/53


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...

JPercivall
In reply to this post by JPercivall
Github user markap14 commented on the pull request:

    https://github.com/apache/incubator-nifi/pull/53#issuecomment-103627150
 
    Nice work. Merged to develop.
   
    Thanks for contributing this back!


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---