stancl / tenancy-docs

stancl/tenancy docs & website
https://tenancyforlaravel.com/docs
MIT License
64 stars 627 forks source link

[4.x] Document `tenants:up` and `tenants:down` commands #214

Closed lukinovec closed 1 year ago

lukinovec commented 2 years ago

I wrote docs for the new Up/Down commands. @stancl, there are comments in the Down command saying some Laravel features are not available with tenants because of storing the data in the tenant database. I think we should document that, but what exactly are the limitations?

EDIT: updated to include info from this comment

The documentation:

Maintenance mode commands (tenant-aware)

Note: Ensure your tenant model uses the MaintenanceMode trait and casts the maintenance_mode property to array before using the Up and Down commands.

This package provides commands for putting tenants into maintenance mode (tenants:down), and out of maintenance mode (tenants:up).

Tenant databases are used for storing maintenance mode information (maintenance_mode in tenant's data column). This cannot be changed by setting the driver using the app.maintenance.driver config key.

The tenants:down command accepts the same options as Laravel's down command, except for --render.

Visit the Laravel documentation for more information about maintenance mode with the mentioned limitations in mind.

php artisan tenants:down

Tenants are now in maintenance mode.

php artisan tenants:up

Tenants are now out of maintenance mode.
stancl commented 2 years ago

No idea about that comment. Can you open the file in the repo and click on "Blame" to find which commit added that?

lukinovec commented 2 years ago

Here's the commit: https://github.com/stein-j/tenancy/commit/b97f409ee3bf669f055a2d9c578afa64d18381fb#diff-9139503e6a6ae12909d9516b90ec1980dcf11275373d8e50ad0872df32f36c0bR42-R44

EDIT: and here's the squashed PR commit https://github.com/archtechx/tenancy/commit/121370ea017e8d0ee541e41417402dc6856445f5

stancl commented 2 years ago

Ah I see, I first thought this is related to migrations (I was reading this on mobile) but this is the maintenance mode. I see what he meant there. By default, Laravel stores maintenance mode details in a file (somewhere in storage_path(), or in storage/framework/... I believe). We store them on each tenant, so we use the tenants table.

Perhaps @stein-j could explain what exact features are not available? Maybe we discussed this in the original PR that I closed, so @lukinovec you could try checking there.

stein-j commented 2 years ago

I must say, my comment is not very helpful, I don't even remember what I meant by "The base down command is heavily used."

Okay, from what I remember there are two things:

1) You can "render" a page, so that Laravel don't even nee to boot and instantly return a static html page (Laravel Doc). This is done in the first file that is loaded [public/index.php] (https://github.com/laravel/laravel/blob/5138bc36dbc884098ea68942e805b2267e7a627f/public/index.php#L19). But with the tenants, the data is stored in the database, so it has to boot Laravel. So the option --render is not available, which I think is fine because if you need to down a single tenant, you probably aren't working on the server at the same time but simply denying access temporally.

2) The second issue is more recent, because Laravel has now implemented drivers for the maintenance mode, so you can decide if you want to store the information that your app is in maintenance mode in the file or in the cache. (mostly useful when using load balancers). So if you change app.maintenance.driver to cache, putting a tenant into maintenance mode will still be stored in the database. Thought, this feature could be replicated in this package.

lukinovec commented 2 years ago

Thank you for the info, I checked out the PR that adds the commands, and it seems the only discussed limitations are the ones you mention here.

Related comments: https://github.com/archtechx/tenancy/pull/761#issuecomment-1001060586 – "all parameters can be recreated except for render because this allows to return a response even before Laravel is initialised (which means, before the package is also initialised)" https://github.com/archtechx/tenancy/pull/761#issuecomment-1001203236 – "It might be important to mention in the documentation that the maintenance mode doesn't uses the driver from the config file"

lukinovec commented 2 years ago

Updated the documentation @stancl

stein-j commented 2 years ago

I've just made a PR so implement tenancy maintenance mode drivers: https://github.com/archtechx/tenancy/pull/967.

Note: This doesn't uses the same config parameter app.maintenance.driver, but has a replicated config in the tenancy file tenancy.maintenance.driver.

Laravel has two drivers file & cache, whereas Tenancy would have database & cache.

I know you just changed the documentation, if this PR is merged you'll need to update it again 😆

stancl commented 2 years ago

Regarding what @stein-j wrote https://github.com/stancl/tenancy-docs/issues/214#issuecomment-1268729429, I think the first issue is fine because the maintenance mode is tenant-specific. And the second issue I addressed in the PR — it's probably not pursuing.

So this looks good now