-
Notifications
You must be signed in to change notification settings - Fork 602
Fix MongoInsertStorage documentation #105
base: master
Are you sure you want to change the base?
Conversation
peterableda
commented
Jun 6, 2014
- Create simple constructor to MongoInsertStorage
- Fix readme documentations and examples
- Add datetime support
* 'master' of github.com:sspinc/mongo-hadoop: Fix usage of MongoInsertStorage and update description Fix usage of MongoInsertStorage * 'master' of github.com:sspinc/mongo-hadoop: Fix usage of MongoInsertStorage and update description Fix usage of MongoInsertStorage
This reverts commit 061016f.
|
||
private final MongoOutputFormat outputFormat = new MongoOutputFormat(); | ||
|
||
public MongoInsertStorage() { | ||
} | ||
|
||
public MongoInsertStorage(final String idField, final String useUpsert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing (or really here you're just renaming) useUpsert is a breaking change. You'll need to add toIgnore as a third parameter.
The inability to store DateTime's to mongo right now is infuriating! I hope this gets accepted. |
My concern with the PR is the "hijacking" of the second string parameter in the MongoInsertStorage constructor. That parameter is currently unused internally (which is probably a bug or at least a partially completed feature) but there's no telling if that constructor is used in the wild. By changing how that String is used, it silently changes (and possibly breaks) anyone who uses that constructor. There are a number of ways to do this PR but I'm really uncomfortable with that constructor change as it is. |
I see your point. I will change that. |
It's an ugly wart (that should have at least been a boolean) but it predates my involvement. :( |
I wanted to issue a pull request for just the datetime conversion but knew that my local changes were possibly overkill, then got hung up trying to get tests to run & pass (I'm a maven user, new to both hadoop and gradle). Will this change also handle datetime fix for updates? Locally I added handling for DateTime in both BSONStorage and MongoStorage (but like I said, may've been overkill). |
Conflicts: pig/README.md pig/src/main/java/com/mongodb/hadoop/pig/MongoInsertStorage.java
@dggrj It should work for updates too. |
@evanchooly I put back the problematic constructor. Please review my code; I will make changes if you say so. I merged master to my fork. Is it ok like this or should I rebase my changes? |
Hi Peter, I am trying to load the data from hdfs to mongo. I was able to do that but I want my own index like group in the below case. But still it is mapping to objectid. The data is loaded to mongo but _id is not apped to group still it is being mapped to objectid. Can you please check below code and let me know if some thing is wrong? Thanks in advance.... SET mapred.job.queue.name edwdev; DEFINE MongoInsertStorage com.mongodb.hadoop.pig.MongoInsertStorage(); |