Skip to content

Conversation

@manveloff
Copy link

@manveloff manveloff commented Dec 5, 2018

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.

…on in the Blueprint.php -> increments, I suggest to comment out exception throwing to allow a migration table creation without errors
@RatkoR
Copy link
Owner

RatkoR commented Dec 5, 2018

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
Copy link
Author

manveloff commented Dec 5, 2018

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
Copy link
Author

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
Copy link
Owner

RatkoR commented Dec 6, 2018

Shortly speaking, don't merge this.

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

@manveloff
Copy link
Author

manveloff commented Dec 6, 2018

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants