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

MongoDB is not mandatory anymore? #364

Closed
Mte90 opened this issue Oct 28, 2020 · 11 comments
Closed

MongoDB is not mandatory anymore? #364

Mte90 opened this issue Oct 28, 2020 · 11 comments
Labels
Milestone

Comments

@Mte90
Copy link
Contributor

Mte90 commented Oct 28, 2020

On VVV now we are using Sqlite but the composer.json of this project and composer.lock require the mongo library.

If mongo is not installed the library block the execution of XHGUI with a warning and blank page, so I had to remove it from the composer.json file to work.

@glensc
Copy link
Contributor

glensc commented Oct 28, 2020

PDO support is not complete, so MongoDB is still the recommended and default datastore:

Please include the exact error and details of your second statement in the issue body.

@Mte90
Copy link
Contributor Author

Mte90 commented Oct 28, 2020

Screenshot_20201028_174055

This is what happens and removing the package fix it.

@glensc
Copy link
Contributor

glensc commented Oct 28, 2020

That message is emitted by composer. And removing what?

@Mte90
Copy link
Contributor Author

Mte90 commented Oct 28, 2020

Ops sorry I forgot the most important part....
Removing alcaeus/mongo-php-adapter from the composper.json and a composer update that warning is gone and I can use XHgui.

@glensc
Copy link
Contributor

glensc commented Oct 28, 2020

You can disable platform check or use composer 1.x:

@Mte90
Copy link
Contributor Author

Mte90 commented Oct 28, 2020

Seems that composer v2 is enabled as true but with false is fixed.

I am trying with composer config platform-check false.

@tomjn
Copy link
Contributor

tomjn commented Nov 2, 2020

Would it make more sense to declare the mongodb adapter package a suggested/recommended dependency? Keeping in mind we already use composer v1.

Also, changing the platform-check option appears to be a Composer v2 feature, this is the result of Composer v1:

vagrant@vvv:/srv/www/default/xhgui$ composer config platform-check false

                                                                             
  [InvalidArgumentException]                                                 
  Setting platform-check does not exist or is not supported by this command  
                                                                             

config [-g|--global] [-e|--editor] [-a|--auth] [--unset] [-l|--list] [-f|--file FILE] [--absolute] [--] [<setting-key>] [<setting-value>]...

To get this working, I had to fork composer.json to add the "platform-check": false option then recreate the autoloader.


The readme, repo description, and documentation claim that PDO is currently a supported option. It is clear that this is not the case. I suggest these be updated to reflect that PDO support is a work in progress.

Our project has wasted a lot of valuable time and lost weeks of progress as a result of this inaccurate information.

@glensc
Copy link
Contributor

glensc commented Nov 4, 2020

Patches in form of pull requests are welcome.

Also, please note that this project is worked on a voluntary basis in the developer's free time and will.

@tomjn
Copy link
Contributor

tomjn commented Nov 4, 2020

As is ours, I would produce a patch myself but I am unfamiliar with the conventions and structure of this repo, nor do I have the administrator access to amend all instances that need adjusting, e.g. the repo description. I also do not know what the appropriate edit would look like.

But in the meantime, please accept constructive feedback constructively. Our project has just lost a significant amount of time due to highly misleading and outright incorrect documentation, and we have a lot of work to do undoing a migration from MongoDB to PDO as a result. As you can appreciate, our capacity and goodwill to contribute changes may be strained.

@glensc
Copy link
Contributor

glensc commented Nov 6, 2020

as noted in the first response in this issue, pdo support is just incomplete:

"incomplete" does not mean outright wrong and pdo not working. what you were doing (removing dependencies) is not intended as out of box solution, so it's not surprising that you end up with troubles.

on the bright side:

there have been fixes continuously to improving pdo support with releases 0.10..0.16:

those fixes are based on user reports and some of the differences came up when trying to setup PHPUnit tests for pdo.

I've tried to map differences in:

so far the "pdo" users have not complained, so I guess most of the wanted functionality is covered. the other pdo than sqlite support was fixed only in 0.15.0 really, but first pdo support was merged in 0.10.0.

@glensc glensc added the question label Nov 6, 2020
@tomjn
Copy link
Contributor

tomjn commented Nov 7, 2020

You have at least 2 PDO users complaining here. I don't mind if using PDO only gets us 90% of the functionality, or if we have to take additional steps, as long as it's clearly stated, which it is not. You yourself reviewed our PR's trying to implement PDO and did not catch this.

You can make whatever decisions you want for your own project, just make sure you document them accurately. I shouldn't have to read through pages of github issues on the off-chance one of them contains relevant information. It's an unreasonable expectation. ( You could even have pinned that issue so it was at the top )

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

No branches or pull requests

3 participants