Add Mongodb driver #94
Conversation
|
Out of curiosity, what would be the advantage of this compared to the other drivers? |
| - MONGO_VERSION=1.3.2 | ||
| - MONGO_VERSION=1.4.4 | ||
|
|
||
| services: mongodb |
There was a problem hiding this comment.
Is it possible to change the test cases to use mocking instead of a real database?
There was a problem hiding this comment.
I've started working on the mocks, but it will be tomorrow before I can get back to them.
|
Great job, have made some comments on the code :) |
|
@GromNaN for me, the advantage is I can use the same db that I'm using for the rest of my site without needing to install a proper queue system until the traffic demands it. |
| { | ||
| $this->db = $db; | ||
| $this->collection = $db->selectCollection('bernardMessages'); | ||
| $this->queueCollection = $db->selectCollection('bernardQueues'); |
There was a problem hiding this comment.
would prefer bernard_messages and bernard_queues as the other drivers. Also i dont know if it is needed to create the collections upfront. What do you think about using $db->bernard_messages when we need the collection ?
There was a problem hiding this comment.
Or instead call the variables $this->messages and $this->queues
There was a problem hiding this comment.
I'm fine with changing the names. If we are not going to create the collections upfront, then we would need to call a method to get the message and queue collections to make sure we have selected the collection.
There was a problem hiding this comment.
This class doesn't seem to use $db at all. Why not just construct it with the actual MongoCollection dependencies? That would (a) allow the user to specify whatever collection names they like and (b) make testing easier, since you could do away with mocking MongoDB::selectCollection() on top of the collections themselves.
There was a problem hiding this comment.
I'm fine with that if @henrikbjorn is ok with me approaching it that way. It is a bit of a PITA to mock MongoDB::selectCollection()
| @@ -26,7 +26,8 @@ | |||
| "symfony/serializer" : "~2.2", | |||
| "doctrine/dbal" : "~2.3", | |||
| "iron-io/iron_mq" : "~1.4", | |||
| "aws/aws-sdk-php" : "~2.4" | |||
| "aws/aws-sdk-php" : "~2.4", | |||
| "ext-mongo" : ">=1.2.12,<1.6-dev" | |||
There was a problem hiding this comment.
Your documentation example uses MongoClient. Honestly, I'd suggest just making this >=1.3.0 so there's no need to even support or test the deprecated Mongo class. The only reason we allow this in doctrine/mongodb is for BC.
There was a problem hiding this comment.
I'm okay with doing this. @henrikbjorn ?
There was a problem hiding this comment.
im okay with requirering >=1.3.0
|
I haven't forgotten this, I've just been pulled away from it for the time being. I'll circle back around to it when I can. |
|
Any news? |
|
Nothing new on this. Are you needing it? If so, I can make it a higher priority on my todo list. |
|
I would be happy to see this merged. |
|
Several people have showed interest in getting this merged. @iampersistent anything new? I have lost track of this driver :) |
|
As I can see it is using the PECL extension. It would be awesome if it could be built like the redis driver: using the extension and a "native" implementation as well. There is a cool one originally created by @FrenkyNet called Monga. Might worth a check. |
|
@sagikazarmark: AFAIK, Monga is just an abstraction around the PECL extension, much like Doctrine MongoDB. A "native" implementation of a MongoDB driver in PHP would be mongofill. |
|
@iampersistent: If you're too busy, I can try to take this on by rebasing and opening a new PR. Shouldn't take more than an hour or two, since you've already done most of the work. Let me know. |
|
@jmikola thanks, I really appreciate you offering to do that. Do what ever you need and ping me if you have any questions. |
|
@jmikola Hi Jeremy :) just popped in to say your assumptions are correct. It's only a "fancy" query builder. |
|
Would it make sense to move this into its own repo? then it could contain drivers for Mongofill, ext-mongo and maybe other simple abstractions? |
|
@henrikbjorn: Mongofill implements the same API as the PECL driver (it defines the same non-namespaced classes), so I don't think we need to write more than a single class. It's up to you if you'd like it in a separate repository though, but I doubt the finished product will look much different than the current PR. |
|
Closed as #133 is now merged, thanks for your work on this anyway :) |
No description provided.