Merge sqlite check to database check #41535
Conversation
come-nc
changed the base branch from
master
to
feat/migrate-database-pending-bigint-conversions
come-nc
force-pushed
the
feat/migrate-database-pending-bigint-conversions
branch
from
73685ec
to
e3d96ff
Compare
come-nc
force-pushed
the
feat/migrate-database-pending-bigint-conversions
branch
from
e3d96ff
to
60bc97e
Compare
come-nc
force-pushed
the
feat/merge-sqlite-check-to-database-check
branch
from
47bf97f
to
09c67c5
Compare
come-nc
added
3. to review
come-nc
requested review from
a team,
ArtificialOwl,
icewind1991 and
sorbaugh
and removed request for
a team
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
come-nc
force-pushed
the
feat/merge-sqlite-check-to-database-check
branch
from
09c67c5
to
95ea46c
Compare
| @@ -81,7 +83,10 @@ public function run(): SetupResult { | |||
| } elseif ($databasePlatform instanceof OraclePlatform) { | |||
| $version = 'Oracle'; | |||
| } elseif ($databasePlatform instanceof SqlitePlatform) { | |||
| $version = 'Sqlite'; | |||
| return SetupResult::warning( | |||
| $this->l10n->t('SQLite is currently being used as the backend database. For larger installations we recommend that you switch to a different database backend. This is particularly recommended when using the desktop client for file synchronisation. To migrate to another database use the command line tool: "occ db:convert-type".'), | |||
There was a problem hiding this comment.
Can we add this warning only if the instance have more than 5 users?
I am thinking about a small arbitrary limit so an admin of a very small instance won't have this warning.
There was a problem hiding this comment.
We can, but should we?
One thing that pops to my mind is that if the admin is setting up for a large instance he will have no users at the beginning and this warning will only be seen once growing, and migration can be hard?
But yeah maybe we can always put the same description, but set the status to success/info/warning depending if user number is <=5/>5/>20?
There was a problem hiding this comment.
https://docs.nextcloud.com/server/latest/admin_manual/installation/system_requirements.html
SQLite (only recommended for testing and minimal-instances)
I think warning is totally fine for "production" instances. It always can have negative performance impact.
We also warn about int32 when you are alone, not having a memcache (info only), missing indexes, etc.
I think warning against SQLite is totally fine as it's really recommended to go with anything else.
Summary
Merge the SQlite setup check into the already existing supported database check.
Checklist