Suggest cli based updater in case the instance is bigger #23922
Conversation
|
works for me. but can we do 50 users? |
|
Please make this setting configurable, not just based on some arbitrary number of users. I want to be able to disable web-based updating on my personal instance, which has <10 users... |
| // count users | ||
| $stats = \OC::$server->getUserManager()->countUsers(); | ||
| $totalUsers = array_sum($stats); | ||
| if ($totalUsers > 10) { |
There was a problem hiding this comment.
issue says 100 not 10
works for me - @karlitschek objections |
|
hmmm. not sure why someone should configure this. if the admin understands this setting then the same admin can choose the right way to update owncloud in the first place. |
|
@karlitschek For me, it's not about telling the admin to perform the update via occ, but rather preventing a random user from performing the update through the web interface, before backups are made etc. |
thinking about the updater page will popup on any user and any random user might unintentionally trigger the upgrade and the admin having no way to interact - this seems legitimate |
|
O.K. Let's do a general 'disable web updater' config option instead. O.K. for everyone? |
default being enabled I assume 😉 |
|
Until recently there was this other PR to implement exactly that... #18782 🙈 |
DeepDiver1975
force-pushed
the
upgrade-only-with-cli-for-big-installations
branch
from
9556fce
to
ccec1d9
Compare
|
Only 3 design inputs:
Ok? |
|
You'll want to check the config value in core/ajax/update.php too, to prevent a malicious user from triggering the update (even if the template page says they can't) and potentially causing problems. |
|
|
I suggest to not differentiate between core and app update in this case but to just say that updates need to be installed - any further detailed information is in the cli update command. |
DeepDiver1975
force-pushed
the
upgrade-only-with-cli-for-big-installations
branch
from
ccec1d9
to
37d7acf
Compare
|
nice. Can we add the reason too? Like: 'because it is disabled in the config.php' or 'because you have a big instance with more then xx users' |
|
Yeah, good call @karlitschek. A reason is good, wording should be like this:
Also, the header could be simplified to »Update needed« (no dot at the end). |
DeepDiver1975
force-pushed
the
upgrade-only-with-cli-for-big-installations
branch
from
37d7acf
to
739dfb5
Compare
|
👍 nice! |
|
👍 |
| @@ -37,9 +37,17 @@ | |||
| // need to send an initial message to force-init the event source, | |||
| // which will then trigger its own CSRF check and produces its own CSRF error | |||
| // message | |||
| $eventSource->send('success', (string)$l->t('Preparing update')); | |||
| //$eventSource->send('success', (string)$l->t('Preparing update')); | |||
ghost
mentioned this pull request
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
refs #23913
@karlitschek
@jancborchardt UX please