fix(CardDAV): only run upgrade sync if 1000 users or less#39911
Merged
fix(CardDAV): only run upgrade sync if 1000 users or less#39911
Conversation
miaulalala
added
bug
2. developing
tcitworld
reviewed
Aug 16, 2023
miaulalala
added
3. to review
Member
ChristophWurst
left a comment
ChristophWurst
left a comment
There was a problem hiding this comment.
nit: append 27 to upgrade_dav_sync_run to allow future dav upgrade syncs to have a unique key too
miaulalala
force-pushed
the
fix/carddav/limit-upgrade-sync-to-1000-users
branch
from
6a7a28e to
0074408
Compare
Contributor
I don't understand 😕 Let's say we need another sync from 28 to 29. If we use the app config key without a prefix/suffix, we can reuse the same logic without changing the command or setup check. |
Member
|
Fair point if we reset upgrade_dav_sync_run. Sorry for the misdirection. |
kesselb
reviewed
Aug 17, 2023
kesselb
reviewed
Aug 17, 2023
Contributor
|
How to test:
|
kesselb
approved these changes
Aug 17, 2023
Contributor
|
/backport to stable27 |
st3iny
approved these changes
Aug 21, 2023
miaulalala
force-pushed
the
fix/carddav/limit-upgrade-sync-to-1000-users
branch
2 times, most recently
from
d6a3589 to
b02c2e5
Compare
kesselb
reviewed
Aug 25, 2023
kesselb
reviewed
Aug 25, 2023
| */ | ||
| public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { | ||
| if($this->userManager->countUsers() > 1000) { | ||
| $this->config->setAppValue('dav', 'needs_system_address_book_sync', 'no'); |
Contributor
There was a problem hiding this comment.
Suggested change
| $this->config->setAppValue('dav', 'needs_system_address_book_sync', 'no'); | |
| $this->config->setAppValue('dav', 'needs_system_address_book_sync', 'yes'); |
miaulalala
force-pushed
the
fix/carddav/limit-upgrade-sync-to-1000-users
branch
from
abec672 to
b8840c6
Compare
kesselb
force-pushed
the
fix/carddav/limit-upgrade-sync-to-1000-users
branch
from
b8840c6 to
7406027
Compare
Signed-off-by: Anna Larch <anna@nextcloud.com>
kesselb
force-pushed
the
fix/carddav/limit-upgrade-sync-to-1000-users
branch
from
7406027 to
7f11ee8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Limit the upgrade sync to instances with 1000 users or less and show a warning in the admin section if the sync hasn't been run:
TODO
Checklist