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

enh: improve occ file:transfer-ownership logging #50663

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Feb 5, 2025

When using occ file:transfer-ownership, we had issues reported that not all files where transferred when using s3 buckets as file storage.

This PR improves the logging of the occ command to inform users when not all files are transferred.

@grnd-alt grnd-alt force-pushed the enh/improve-transfer-ownership-logging branch from 77d0617 to 3b85ca0 Compare February 5, 2025 12:13
@grnd-alt grnd-alt force-pushed the enh/improve-transfer-ownership-logging branch from 3b85ca0 to c00b9bd Compare February 5, 2025 13:05
@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews labels Feb 6, 2025
@juliusknorr juliusknorr added this to the Nextcloud 32 milestone Feb 6, 2025
@juliusknorr
Copy link
Member

@grnd-alt Please assign labels on new prs and provide a meaninful description, especially for reviewers that have not been involved before this will speed up review as it is easier to understand the context

@juliusknorr
Copy link
Member

Also conventional commit message should be feat not fix I'd say

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current wording is not clear enough I think.

apps/files/lib/Service/OwnershipTransferService.php Outdated Show resolved Hide resolved
@grnd-alt grnd-alt force-pushed the enh/improve-transfer-ownership-logging branch 2 times, most recently from c18ed34 to f8b7509 Compare February 6, 2025 10:45
Signed-off-by: grnd-alt <github@belakkaf.net>
@grnd-alt grnd-alt force-pushed the enh/improve-transfer-ownership-logging branch from f8b7509 to a1fd916 Compare February 11, 2025 07:59
@come-nc
Copy link
Contributor

come-nc commented Feb 13, 2025

I’m afraid of the performance impact of this.
For source size it may have it in cache, but for the destination user it will force a full scan, no?
Also, the source size may not be perfectly up to date in the cache and this will result in a false-positive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants