-
Notifications
You must be signed in to change notification settings - Fork 82
Upgrade to Mongo 2.0 Syntax #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
|
||
| it "should return a db" do | ||
| @mongodb_connection.db.should be_a Mongify::Database::NoSqlConnection::DB | ||
| @mongodb_connection.db.should be_a Mongify::Database::NoSqlConnection::Database |
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.
Line is too long. [85/80]
| it "should drop the index" do | ||
| @collection.should_receive(:drop_index).with('pre_mongified_id_1') | ||
| @collection.stub(:update) | ||
| @indexes.should_receive(:drop_one).with('pre_mongified_id_1') |
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.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| it "should call update with unset" do | ||
| @collection.should_receive(:update).with({},{'$unset' => {'pre_mongified_id' => 1}}, {:multi=>true}) | ||
| @collection.stub(:drop_index) | ||
| @collection.should_receive(:update_many).with({},{'$unset' => {'pre_mongified_id' => 1}}) |
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.
Space missing after comma.
Space inside { missing.
Redundant curly braces around a hash parameter.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [97/80]
Space inside } missing.
| @indexes.stub(:drop_one) | ||
| @collection.stub(:indexes).and_return(@indexes) | ||
| @collection.stub(:update_many) | ||
| @indexes.stub(:get).and_return('pre_mongified_id_1' => 'something') |
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.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| it "should create index for pre_mongified_id" do | ||
| @collection.should_receive(:create_index).with([["pre_mongified_id", Mongo::ASCENDING]]).and_return(true) | ||
| @collection.indexes | ||
| @collection.indexes.should_receive(:create_many).with([ { key: { pre_mongified_id: 1 } } ]).and_return(true) |
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.
Space inside square brackets detected.
Line is too long. [114/80]
| id = row["_id"] | ||
| attributes = fetch_reference_ids(t, row) | ||
| no_sql_connection.update(t.name, id, {"$set" => attributes}) unless attributes.blank? | ||
| no_sql_connection.connection[t.name].update_one( { "_id" => id } , { "$set" => attributes}) unless attributes.blank? |
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.
Space inside parentheses detected.
Space found before comma.
Redundant curly braces around a hash parameter.
Line is too long. [129/80]
Unnecessary spacing detected.
Operator => should be surrounded by a single space.
Space inside } missing.
| # speedy lookup for collections via the pre_mongified_id | ||
| def create_pre_mongified_id_index(collection_name) | ||
| db[collection_name].create_index([['pre_mongified_id', Mongo::ASCENDING]]) | ||
| db[collection_name].indexes.create_many([ { key: { pre_mongified_id: 1 } } ]) |
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.
Space inside square brackets detected.
Line is too long. [85/80]
| # @raise MongoDBError if index isn't found | ||
| def drop_mongified_index(collection_name) | ||
| db[collection_name].drop_index('pre_mongified_id_1') if db[collection_name].index_information.keys.include?("pre_mongified_id_1") | ||
| db[collection_name].indexes.drop_one('pre_mongified_id_1') unless db[collection_name].indexes.get('pre_mongified_id_1').nil? |
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.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [132/80]
| def remove_pre_mongified_ids(collection_name) | ||
| drop_mongified_index(collection_name) | ||
| db[collection_name].update({}, { '$unset' => { 'pre_mongified_id' => 1} }, :multi => true) | ||
| db[collection_name].update_many({}, { '$unset' => { 'pre_mongified_id' => 1} }) |
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.
Redundant curly braces around a hash parameter.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [87/80]
Space inside } missing.
| # Returns a row of a item from a given collection with a given pre_mongified_id | ||
| def get_id_using_pre_mongified_id(colleciton_name, pre_mongified_id) | ||
| db[colleciton_name].find_one('pre_mongified_id' => pre_mongified_id).try(:[], '_id') | ||
| db[colleciton_name].find('pre_mongified_id' => pre_mongified_id).try(:[], '_id') |
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.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [88/80]
| context "get_id_using_pre_mongified_id" do | ||
| it "should return new id" do | ||
| @collection.should_receive(:find_one).with({"pre_mongified_id"=>1}).and_return({'_id' => '123'}) | ||
| @collection.should_receive(:find).with({"pre_mongified_id"=>1}).and_return([{'_id' => '123'}]) |
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.
Space inside { missing.
Redundant curly braces around a hash parameter.
Surrounding space missing for operator =>.
Space inside } missing.
Line is too long. [102/80]
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| # Returns a row of a item from a given collection with a given pre_mongified_id | ||
| def get_id_using_pre_mongified_id(colleciton_name, pre_mongified_id) | ||
| db[colleciton_name].find_one('pre_mongified_id' => pre_mongified_id).try(:[], '_id') | ||
| db[colleciton_name].find('pre_mongified_id' => pre_mongified_id).try(:first).try(:[], '_id') |
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.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [100/80]
|
Hey @yuvalkarmi, |
|
Hey @anlek , i hope that you upgrade Mongify as soon as possible to work with MongoDB 3.4, also, i suggest you provide a support for SSL configuration in database.config. and what about replicaSet and authSource in the previous connection string? Thank you and Best Regards, |
|
hi @yuvalkarmi , |
|
hey @doltiana -- so as @anlek mentioned, there are still a few tweaks to make before this can be pulled into core (notably, some tests are failing). However, the core functionality is working, and you can try it (at your own risk). For now, you can simply use my fork to do it, until we get everything sorted out: https://github.com/yuvalkarmi/mongify There's an updated readme that outlines support for MongoDB 3.4 and SSL. I'll take care of the outstanding issues as soon as I get a bit of free time, and then Andrew would be able to merge it into core. Happy to answer any other questions you may have, |
|
hey @yuvalkarmi, or updating Gemfile , or what exactly? Thank You. |
|
@doltiana if you pull the code, you can follow the instructions under "Development" in the README. |
|
Hey @anlek , can you provide me with specific instructions of how to pull the mongify gem from the fork by @yuvalkarmi , i tried different ways i found online but nothing seems to work. like currently in my system i have installed mongify using: gem install mongify |
|
@doltiana you can As for the remote MongoDB, I would recommend testing on a local copy as remote has network delays that add up fast! |
|
Hi Gents. Hope all is well. I found out that mongoDB 3.6 is in the horizon and was wondering if the changes being done in mongify will also support this new release. Thanks! |
|
Unfortunately, at the moment, I'm unable to put time into Mongify, but I am open to reviewing people's code and merging it in. |
I've gone ahead and made the necessary changes in across the board for this gem to work with MongoDB versions 3.4+, which have not been previously supported.
I've also added support for sharded clusters, and the ability to use SSL when connecting.
Please see the full list of updates in the changelog.
One note -- I did not test the sync ability or connection to sqlite3 databases.
The setup I did test this with was MySQL + MongoDB 3.4
You're more than welcome to edit this as needed.
Thanks for making this gem, and I hope my contribution helps! :)