CRUD
CRUD copied to clipboard
Laravel-Backpack
Create a Schema Manager to avoid huge loading infering database information
WHY
BEFORE - What was wrong? What was happening before this PR?
All started in https://github.com/Laravel-Backpack/CRUD/issues/4618 when a user reported HUGE loading times for CRUDS.
At that time I was not able to reproduce the issue, but a few days ago that SAME issue starting hunting me.
I have a suspect that is dbal that introduced this since I didn't change nothing in the development environment apart from updating packages.
Testing around the stuff it seems that it's a problem in mysql 5.7 atleast, and affect MariaDB users too (since it was branched from mysql 5.x). Upgrading to mysql 8 solved the issue instantaneously.
AFTER - What is happening after this PR?
We hold the database schema on the SchemaManager and all subsquent calls are made to this object instead of the db directly.
mysql 5.7 without this branch
mysql 5.7 with this branch
mysql 8 with this branch
mysql 8 without this branch
Is it a breaking change?
I tried to make it non breakable ... in the next major all crud and model methods should be moved to the manager and probably create adapters for each db engine we would like to support.
@realtebo This is the solution me and @tabacitu came up after finding the problem for the huge loading times you reported in #4618
If you have time and don't mind, would you give this branch a test to see if it fixes the issues you reported ?
Cheers
Yup - @realtebo or anybody else who can test this. Let us know if everything works ok after doing:
composer require backpack/crud:"dev-hold-the-schema as 5.3.99"
If so, we'll merge and tag this, and then you can do composer require backpack/crud:"^5.3" again, to get the official, launched version.
I am actually away from home and from work. And so o am happily away from pc.. I will try you works in a week . Thanks in advance
I am actually away from home and from work. And so o am happily away from pc.. I will try you works in a week . Thanks in advance
I wish you an happy vacations. We spend alot of time in computer, spending 1/2 weeks away is a bless! :pray:
I really wish we could work for 2 weeks and do what we like to do for the next two. If you like to code, so be it, but it's what you like to do, not mandated, so watching birds is what you like ? Do it.
We shouldn't commit so much time from our lifes to work, it's counter "living"!
See you next week :tropical_drink:
Cheers
I only have one question for you here @pxpm
Other than that, I'm very VERY happy with this. If @jorgetwgroup gave it a test too and we have his thumbs up... I'm ready to merge it. I love it ❤️ So clean!
(or maybe @munjaldevelopment is a better tester for this, since he was using MariaDB anyway? you decide please @pxpm 🙏)
@tabacitu all fixed.
Problem was that in testing we use the same connection to test regular tables and tables with prefix.
Since I didn't separate that tests when I wrote them, the schema for the testing connection was generated before the prefixed tables were created, and it would not regenerate (as expected).
I thought about moving the prefixed database tests to other testing environment, but decided to "fix" it in DatabaseSchema. Basically it now checks the connection as before, but if the connection already has a schema it tries to get only the specific table.
So even if you are creating tables during the app lifecycle (tenants or something like) we make sure we get the tables after the schema was generated.
Please allow the tests to run in this branch so we are sure everything is working fine!
2 beers can't hold me back! muhahahaha 🙃 !
Cheers
Sorry for VEEERY late reply.
I can confirm that upgrading mysql from 5.7 to 8 fix EVERY AND EACH slowness problem.
@tabacitu moved the files, applied your suggestions, let me know if was something like that you had in mind.
Cheers
Update: found a big bug - https://github.com/Laravel-Backpack/CRUD/issues/4704