-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Use PHP ext mongodb instead of mongo for v3 #23
Comments
Did you try https://github.com/alcaeus/mongo-php-adapter? Originally posted by @Maks3w at zendframework/zend-session#33 (comment) |
I didn't try it but I think we should update the handler. I can bring a PR if it is accepted. Originally posted by @sandrokeil at zendframework/zend-session#33 (comment) |
@sandrokeil - please do! Also, we should definitely target the official wrapper, and not arbitrary third party wrappers. Originally posted by @weierophinney at zendframework/zend-session#33 (comment) |
Maybe it's better to not rely on mongo-php-library and use the low level functions of the MongoDB Driver. It's more work, but we have a dependency fewer. I will update my PR depending on the decision. Originally posted by @sandrokeil at zendframework/zend-session#33 (comment) |
I prefer remove the support to the old mongo driver and focus the efforts only in the new mongodb extension. But this is not BC, /fw @weierophinney Originally posted by @Maks3w at zendframework/zend-session#33 (comment) |
@Maks3w ext/mongo has been deprecated, and, more importantly, does not support PHP 7. We need to migrate away from it. Additionally, the page linked by @sandrokeil says specifically:
I'd argue the code in this component (and other ZF components) falls directly into that category. As such, the requirements should be:
Chances are, if a developer is using mongo already in their application, and using ext/mongodb, they will be using the MongoDB PHP library anyways, so this will not be an additional dependency for them, and will help simplify our own code. Originally posted by @weierophinney at zendframework/zend-session#33 (comment) |
My point is we fall under the same scenario of removing zend-crypt from zend-mail. If we remove the support for ext/mongo this should happen on next major version because composer won't alert about the change in the dependency. Old users must install the new extension. Originally posted by @Maks3w at zendframework/zend-session#33 (comment) |
@Maks3w Not true; composer honors extension constraints. As such, if the extension is missing when you run Originally posted by @weierophinney at zendframework/zend-session#33 (comment) |
There aren't required extensions defined https://github.com/zendframework/zend-session/blob/master/composer.json#L15-L18 And I guess we won't add mongodb because is only required for only one specific savehandler. May we could try to add the old extension as a conflicted dependency. But many other libraries may require the old extension. Originally posted by @Maks3w at zendframework/zend-session#33 (comment) |
What's about to use a class alias like in prior versions of stdlib? Originally posted by @sandrokeil at zendframework/zend-session#33 (comment) |
The
Zend\Session\SaveHandler\MongoDB
should use the new MongoDB extension which runs also under PHP 7 and should not rely on the deprecated extension mongo which is not compatible with PHP 7.I think it should based on the official MongoDB PHP library. We can add this library to the suggest Composer section.
What do you think?
Originally posted by @sandrokeil at zendframework/zend-session#33
The text was updated successfully, but these errors were encountered: