You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by tequalsme <gi...@git.apache.org> on 2015/05/08 20:21:47 UTC
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
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 <te...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by bjames301 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by tequalsme <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by joewitt <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by tequalsme <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by tequalsme <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by markap14 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by joewitt <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by tequalsme <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by brianghig <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by brianghig <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by markap14 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by bjames301 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-nifi pull request: NIFI-570: Added MongoDB put and get p...
Posted by tequalsme <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---