Skip to content
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

Support driver options in configuration #397

Merged
merged 3 commits into from
Feb 19, 2017

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jan 4, 2017

Fixes #380. With the release of doctrine/mongodb 1.4 it is possible to pass driver options to the underlying MongoConnection class. This allows users to specify a stream context containing options for SSL connections or logging.

The stream context must be declared as a service and the service name given in the configuration for the connection.

@alcaeus alcaeus added the Feature label Jan 4, 2017
@alcaeus alcaeus added this to the 3.3 milestone Jan 4, 2017
@alcaeus alcaeus requested a review from malarzm January 4, 2017 10:02
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 but please include docs :)

@alcaeus alcaeus force-pushed the driver-options-support branch from 138e34d to 74a8905 Compare February 12, 2017 06:12
@alcaeus
Copy link
Member Author

alcaeus commented Feb 12, 2017

@malarzm finally done. While writing documentation, I noticed that it's a little ugly to create a service for the stream context, since the service definition needs a class name when it actually returns a scalar type:

services:
    app.mongodb.context:
        class: 'resource'
        factory: 'stream_context_create'
        arguments:
            - { ssl: { verify_expiry: true } }

It works just fine, it just isn't pretty.

@malarzm
Copy link
Member

malarzm commented Feb 19, 2017

Thanks for the docs!

@malarzm malarzm merged commit 1115db4 into doctrine:master Feb 19, 2017
@peshi
Copy link

peshi commented Feb 19, 2017

@alcaeus @malarzm So, which one is it - doc's say driver_options while it infact is driverOptions :)
It also seems to be required to add this in the config otherwise it will throw

Catchable Fatal Error: Argument 5 passed to Doctrine\MongoDB\Connection::__construct() must be of the type array, null given

@The-Don-Himself
Copy link

Yeah, getting the same fatal error issue as @peshi above after update.

@alcaeus alcaeus deleted the driver-options-support branch February 20, 2017 06:27
@alcaeus
Copy link
Member Author

alcaeus commented Feb 20, 2017

Thanks for the bug reports, I'll be fixing this shortly. In the meantime, please change your composer.json to use a stable release: "doctrine/mongodb-odm-bundle": "^3.2.1",. This will also allow installing the 3.3 release with driverOptions support once it's stable.

@alcaeus
Copy link
Member Author

alcaeus commented Feb 20, 2017

@peshi I got confused while writing the docs. I'll be changing it to driver_options (driver-options in XML) to be consistent to the rest of the parameter naming. Only MongoDB options are camel-cased, all others are snake-cased.

@alcaeus
Copy link
Member Author

alcaeus commented Feb 20, 2017

@peshi @The-Don-Himself I've fixed this in master, so there should be no discrepancies between code and docs and also no fatal error due to me not understanding types (😉 ).

@The-Don-Himself
Copy link

@alcaeus thanks for the hard-work! Its doesn't always have to be perfect, even Einstein got E = mc2 wrong several times before he could prove it. That said, you should probably re-check your coffee supply :)

@alcaeus
Copy link
Member Author

alcaeus commented Feb 20, 2017

@The-Don-Himself checked and fixed 👍

@peshi
Copy link

peshi commented Feb 20, 2017

@alcaeus No worries :) Thanks for the quick response and fix! 👍

@sogetimaitral
Copy link

Hello,

How can I use the options while waiting for a stable version? I'm totally stuck in a cloud environment where Mongo comes with SSL, with no way to disable it. The driver_options is not recognized yet (I'm on the stable release), even though the documentations give the configuration to do it.

Should I use the dev-master? I'd rather not, since there's a risk for a prod environment.
But the 3.2.x-dev branch seems old and does not include the feature...

Thanks!

@alcaeus
Copy link
Member Author

alcaeus commented Apr 10, 2017

@sogetimaitral in that case, I'd suggest you use dev-master. I haven't had a lot of time in the past few weeks to take care of open issues, but I'll try to cut the 3.3 release ASAP.

Looking at the 3.3 milestone, the only feature I'd definitely want in 3.3 is handling aggregation queries in the data collector, since we'll bring in aggregation builder with the upcoming ODM release.

I'll see if I can build that data collector functionality soon and release 3.3 shortly thereafter.

@sogetimaitral
Copy link

Thanks @alcaeus this is what I did, and it's working, but I'm not really at ease using a dev branch in a prod environment.
We will wait for a release, we still have some time (a matter of days, up to 2 weeks). If you see that it's taking too much time on your side, we could propose a pull request on the 3.2 branch to create a 3.2.2 version for this fix only? The merge seems to be easy.

@alcaeus
Copy link
Member Author

alcaeus commented Apr 10, 2017

Unfortunately, this isn't a fix, it's a new feature. Thus, according to Semantic Versioning which we follow, it requires a new minor release. Looking at the diff between 3.2 and dev-master, you shouldn't feel too bad about using a dev version - the changes are exclusively new features and docs updates.

Still, I'll see to release 3.3 in the next couple of weeks. Maybe I can put the long weekend to good use.

@sogetimaitral
Copy link

OK, I agree with your use of semantic versioning. Sorry for the extra work then... :(

@alcaeus
Copy link
Member Author

alcaeus commented Apr 10, 2017

No worries - I should've gotten to it a while ago 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants