MongoDB storage driver #133
Conversation
This includes all commits from #94.
This commit builds upon @iampersistent's work in #94. The driver is now constructed with MongoCollection objects, which removes the previous MongoDB::selectCollection() logic. Additionally, the message sentAt field is now a BSON date and queue name was added to acknowledgeMessage()'s remove() criteria to be consistent with the SQL driver.
| before_script: | ||
| - sh -c "if [ \"$TRAVIS_PHP_VERSION\" != \"hhvm\" ]; then pyrus install pecl/redis && pyrus build pecl/redis; fi" | ||
| - sh -c "if [ \"$TRAVIS_PHP_VERSION\" != \"hhvm\" ]; then echo \"extension=redis.so\" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi" | ||
| - sh -c "if [ \"$TRAVIS_PHP_VERSION\" == \"hhvm\" ]; then composer require --dev mongofill/mongofill=dev-master --no-update; fi" |
There was a problem hiding this comment.
Based on the Travis CI logs (see here), this is not taking effect on HHVM. Any ideas?
There was a problem hiding this comment.
this is because your command is invalid: https://travis-ci.org/bernardphp/bernard/jobs/43233639#L37
It should be =, not ==
There was a problem hiding this comment.
btw, I suggest running it as ``sh -e -c`, so that failures in this command make the build fail (or remove the sh wrapper entirely as pasting the command itself in the sh build script generated by Travis will work too)
There was a problem hiding this comment.
Thanks for pointing out the operator error. As for the -e, mongofill is an optional dependency (like the redis extension), so I'm going to leave that as is. @henrikbjorn should probably consider changing all of them, though.
|
@henrikbjorn: It looks like some of the HHVM test failures are due to test cases taking longer than the allotted time (PHPUnit is defaulting them to one second due to strict mode). Perhaps some of these should be annotated differently, as discussed in this Stack Overflow answer? |
|
Annotate away :) but it seems like the increase is time is recent, maybe because of a new hhvm version. |
| { | ||
| $this->messages->remove(array( | ||
| '_id' => new MongoId((string) $receipt), | ||
| 'queue' => (string) $queueName, |
There was a problem hiding this comment.
Do you really need the queue if you have the id?
There was a problem hiding this comment.
No, but it's basically a free check since the _id index will be used anyway. Since we do take both arguments, I think it's best to actually use them. This avoids the odd case where a user acknowledges a message but gets the queue name wrong.
|
@henrikbjorn: Travis is passing now. I think this is good to merge. |
|
Thanks :) |
|
This is awesome. |
|
Ill try and get time for a new release soon, there are some changes and unfortunately there will be some more. |
This supersedes #94 (cc: @iampersistent).