search

RatkoR / laravel-crate.io

Crate.io driver for Laravel
MIT License
36 stars 12 forks source link

A migration table in laravel is not creating because of throw excepti… #23

Open manveloff opened 5 years ago

manveloff commented 5 years ago

I've installed Laravel 5.7.16 and laravel-crate.io 7.0.0. End an empty database. Then I want to create a migration table and invoke my migrations. I type: php artisan migrate:refresh --seed And get the error: Auto increments are not supported in Crate.io

A migration table in laravel is not creating because of throwing an exception in the Blueprint.php -> increments, I suggest to comment out exception throwing to allow a migration table creation without errors.

RatkoR commented 5 years ago

Yes, I've got to this one also. What I did is I created migration table manually and then didn't think about it any more.

I can't just remove the exception. It's there on purpose, to guard that users don't use autoincrements in their migrations. Maybe a better way would be to override migration table create procedure and create it without auto increments.

manveloff commented 5 years ago

This problem may cause troubles to new users of the package. And we can't deploy without manual creation of migrations table, it's not convenient. Why shouldn't we automate a solution?

I've commited some changes, so we can kill two birds with one stone - save the exception throwing and avoid the increments error during migrations creation.

We have this problem in the createRepository method on the line 154 in the class "vendor/laravel/framework/src/Illuminate/Database/Migrations/DatabaseMigrationRepository.php"

To solve it, we could be using the debug_backtrace() in the increments method in Blueprint.php. If we see createRepository in the debug backtrace, we can not to throw an exception.

manveloff commented 5 years ago

I was thinking about that - you right, we can't just ignore increments error. I've looked at migrations table in case of ignoring, and there is no 'id' column. I think that is bad sign.

Shortly speaking, don't merge this.

RatkoR commented 5 years ago

Shortly speaking, don't merge this.

Ok, I'll keep the PR open so that I don't forget about this problem. Thx!

manveloff commented 5 years ago

Just some my thoughts.

In addition to migrations, I was faced with difficulties in many other places. Maybe we shouldn't use Crate for Laravel and third party packages which use database. Because they are don't take into account Crate's special features, and their code doesn't work properly. And it will be madness to make fixed forks of all these packages and maintain all this stuff.

For now I thing it will be better to use ordinary DB (like MySQL) for Laravel and third party packages, and use Crate for your app's tables (as a rule, application tables require horizontal scaling and contain most of the data).