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

tenant migration adds to landlord database #69

Closed
aminraeisi opened this issue Jun 30, 2020 · 16 comments
Closed

tenant migration adds to landlord database #69

aminraeisi opened this issue Jun 30, 2020 · 16 comments

Comments

@aminraeisi
Copy link

Hi,

Thanks for the great package!

I just started testing the package and created some records directly into the landlord.tenants table and ran php artisan tenants:artisan migrate but it adds them to my landlord database (only once).

here is my files and the settings I am using:

// config/database.php

return [

    'default' => env('DB_CONNECTION', 'landlord'),


    'connections' => [

        'tenant' => [
            'driver' => 'mysql',
            'host' => env('DB_HOST', '127.0.0.1'),
            'port' => env('DB_PORT', '3306'),
            'database' => null,
            'username' => env('DB_USERNAME', 'forge'),
            'password' => env('DB_PASSWORD', ''),
            'unix_socket' => env('DB_SOCKET', ''),
            'charset' => 'utf8mb4',
            'collation' => 'utf8mb4_unicode_ci',
            'prefix' => '',
            'prefix_indexes' => true,
            'strict' => false,
            'engine' => null,
            'options' => extension_loaded('pdo_mysql') ? array_filter([
                PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'),
            ]) : [],
        ],

        'landlord' => [
            'driver' => 'mysql',
            'host' => env('DB_HOST', '127.0.0.1'),
            'port' => env('DB_PORT', '3306'),
            'database' => 'landlord',
            'username' => env('DB_USERNAME', 'forge'),
            'password' => env('DB_PASSWORD', ''),
            'unix_socket' => env('DB_SOCKET', ''),
            'charset' => 'utf8mb4',
            'collation' => 'utf8mb4_unicode_ci',
            'prefix' => '',
            'prefix_indexes' => true,
            'strict' => false,
            'engine' => null,
            'options' => extension_loaded('pdo_mysql') ? array_filter([
                PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'),
            ]) : [],
        ],
]
// config/multitenancy.php

return [

    'tenant_finder' => DomainTenantFinder::class,

    'tenant_artisan_search_fields' => [
        'id',
    ],

    'switch_tenant_tasks' => [
        Spatie\Multitenancy\Tasks\SwitchTenantDatabaseTask::class,
    ],


    'tenant_model' => Tenant::class,

    'queues_are_tenant_aware_by_default' => true,

    'tenant_database_connection_name' => 'tenant',

    'landlord_database_connection_name' => 'landlord',

    'current_tenant_container_key' => 'currentTenant',

    'actions' => [
        'make_tenant_current_action' => MakeTenantCurrentAction::class,
        'forget_current_tenant_action' => ForgetCurrentTenantAction::class,
        'make_queue_tenant_aware_action' => MakeQueueTenantAwareAction::class,
        'migrate_tenant' => MigrateTenantAction::class,
    ],
];
# .env
DB_HOST=localhost
DB_PORT=3306
DB_USERNAME=homestead
DB_PASSWORD=secret
@masterix21
Copy link
Collaborator

The problem could be your default connection. Try with php artisan tenants:artisan "migrate --connection=tenant" an give me a feedback.

@aminraeisi
Copy link
Author

@masterix21 thanks for the quick reply!
Yes I think it is with switching the connection because it adds all the tables (except telescope with error) but in the wrong place.
I just ran php artisan tenants:artisan "migrate --connection=tenant" and it give me the error The "--connection" option does not exist.

@masterix21
Copy link
Collaborator

sorry, my mistake. Try php artisan tenants:artisan "migrate --database=tenant"

@masterix21
Copy link
Collaborator

Did it solve your problems?

@aminraeisi
Copy link
Author

@masterix21 I think this was also related to my other issue with telescope here.

So here is what I did:

  1. Ignore telescope migrations inside AppServiceProvider and publish default telescope migrations as mentioned here (move it to landlord migrations folder).
  2. Inside telescope.php use the default connection set to landlord 'connection' => env('DB_CONNECTION', 'landlord').
  3. Run the command you mentioned with the database param php artisan tenants:artisan "migrate --database=tenant"

And it worked as expected but now I want to know why it does not work without the database param because it supposed to know that the connection is for tenants when I run the command starting with php artisan tenants:, right?

@masterix21
Copy link
Collaborator

masterix21 commented Jul 1, 2020

Without the database parameter, Laravel will use your default connection (specified in your .env file DB_CONNECTION=)

@aminraeisi
Copy link
Author

I am doing php artisan tenants:artisan migrate just like Freek did in this video

Running it with the fresh also drops all tables in landlord and create them again for many times (depending how many tenants I have)

@masterix21
Copy link
Collaborator

masterix21 commented Jul 1, 2020

yes, I know. Trust me, it is better to specify the database in your migration command. You can also specify a path to separate your tenant migrations from landlord migrations.

For example, you could have these directories:
database/migrations/landlord/
database/migrations/tenant/

And doing it:
`php artisan tenants:artisan "migrate --database=tenant --path=database/migrations/tenant"

It is a way to keep your code well organized.

@freekmurze
Copy link
Member

I haven't looked at this issue in detail, but I welcome improvements to our docs to guide people better around how migrations should be handled.

@masterix21
Copy link
Collaborator

Ok, I will do it

@aminraeisi
Copy link
Author

aminraeisi commented Jul 1, 2020

I totally agree with you. Even if I solve it, I will be running my production migrations with the path and database params but it is a nice to have thing 😄

Thanks again for your help. I think it is better to have a github page linking to different gist for different tasks and that way this package will be easier to pickup for beginners like me (SwitchTenantTelescopeTask, SwitchTenantS3BucketTask, etc). I will be happy to build that but I think @freekmurze or @masterix21 have to get the credit for the amazing work.

@masterix21
Copy link
Collaborator

@freekmurze does all dirty job: I am only a contributor like could be you too. Thank you.

@aminraeisi
Copy link
Author

I totally agree with you. Even if I solve it, I will be running my production migrations with the path and database params but it is a nice to have thing

Thanks again for your help. I think it is better to have a github page linking to different gist for different tasks and that way this package will be easier to pickup for beginners like me (SwitchTenantTelescopeTask, SwitchTenantS3BucketTask, etc). I will be happy to build that but I think @freekmurze or @masterix21 have to get the credit for the amazing work.

Adding it to the docs is also good but it will make users think that these tasks are suggested by this package which is not and the package wants to give freedom to users to make their own tasks how they want it.

@freekmurze
Copy link
Member

Thanks all for you feedback/work on this package 👍

@masterix21
Copy link
Collaborator

Finally, I can confirm the error. See the attached screenshots.

migrate:fresh without --database=
migrate-fresh-without-database

migrate:fresh with --database=
migrate-fresh-with-database

Using "without" will reset your landlord database. Now, I'm upgrading the documentation.

@aminraeisi
Copy link
Author

Thanks!

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

No branches or pull requests

3 participants