Skip to content

Implement remove command #89

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 81 commits into
base: develop
Choose a base branch
from
Open

Implement remove command #89

wants to merge 81 commits into from

Conversation

rk1274
Copy link
Contributor

@rk1274 rk1274 commented Feb 12, 2025

@rk1274 rk1274 requested review from sb10 and mjkw31 February 12, 2025 15:37
mjkw31
mjkw31 requested changes Feb 12, 2025
View reviewed changes
sb10
sb10 requested changes Feb 13, 2025
View reviewed changes
@y-popov
Copy link
Contributor

y-popov commented Feb 13, 2025

We also need to store remove requests in the database, so if a server goes down we will be able to recover interrrupted removal process

@rk1274 rk1274 requested a review from sb10 February 17, 2025 13:26
sb10
sb10 requested changes Feb 17, 2025
View reviewed changes
mjkw31
mjkw31 requested changes Feb 18, 2025
View reviewed changes
sb10
sb10 requested changes Feb 18, 2025
View reviewed changes
Copy link
Contributor

@sb10 sb10 left a comment

Choose a reason for hiding this comment

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

I think you're still going through my previous review, but here are some more things I noticed.

@rk1274 rk1274 force-pushed the remove-command branch 4 times, most recently from 85750a7 to 78ed27e Compare February 21, 2025 16:05
mjkw31
mjkw31 requested changes Mar 4, 2025
View reviewed changes
@rk1274 rk1274 requested review from sb10 and mjkw31 March 6, 2025 16:21
mjkw31
mjkw31 requested changes Mar 6, 2025
View reviewed changes
sb10
sb10 requested changes Mar 7, 2025
View reviewed changes
mjkw31
mjkw31 requested changes Mar 12, 2025
View reviewed changes
mjkw31
mjkw31 reviewed Mar 14, 2025
View reviewed changes
sb10
sb10 requested changes Mar 14, 2025
View reviewed changes
@rk1274 rk1274 requested a review from sb10 March 18, 2025 08:59
mjkw31
mjkw31 requested changes Mar 18, 2025
View reviewed changes
mjkw31
mjkw31 previously approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@mjkw31 mjkw31 left a comment

Choose a reason for hiding this comment

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

LGTM.

sb10
sb10 requested changes Mar 18, 2025
View reviewed changes
@rk1274 rk1274 requested a review from sb10 March 28, 2025 15:56
sb10
sb10 previously approved these changes Mar 31, 2025
View reviewed changes
rk1274 and others added 26 commits April 9, 2025 13:38
@rk1274
Rename and polish removeDiscoverLinker(); fix PR comments
07bf994
@rk1274
Handle rare inode edge cases WIP
e6bbcb7
@rk1274
Handle rare inode edge cases
7f146ad
@rk1274 @y-popov
Replace sync.Map with a regular map inside discoveryCoordinator
7f55d46 
Co-authored-by: Yaroslav <[email protected]>
@rk1274
Small optimisation for LoadOrStore()
083700c
@rk1274
PR fixes
33ffb1e 
- Rename functions in discoveryCoordinator
- Move buildRemoveItemDef to set pkg
- Make mock mutex private
@rk1274
Add further tests for discovery coordinator; fix PR comments
71a0aba
@rk1274
Fix race condition
306a1fb
@rk1274
Add logging and ignore unexpected errors when removing dirs
79a975c
@rk1274
Reset removal values when readding a set
935b68f
@rk1274
Add check for if a sub bucket exists before trying to remove it
755f03d
@rk1274
Add check in removeEntry for if the bucket exists before using it
aa24cd6
@rk1274
remove discovered folders from the database during removal of a base …
1050a14 
…folder
@rk1274
Don't count duplicate files when removing
385a20a
@rk1274
Handle removals for files that failed to upload
b46e4c6
@rk1274
Do not allow removals of pending files
64f8363
@rk1274
Remove from failed bucket during removal
7551d5a
@rk1274
Make removal work on sets created with old code
4f555ec
@rk1274
Finalise issues with sets made with old code and finish issues with i…
622bd2b 
…ncomplete sets
@rk1274
Delint
ce96bb9
@rk1274
Add DirRemovalError struct
9120cd8
@rk1274
Fix UpdateRemoveRequest for directories
4a620ef
@rk1274
Add DirNotEmptyError struct
f6fa8cd
@rk1274
Reset sizeRemoved when readding a set
f6f4be8
@rk1274
Add errs pkg
6e17531
@rk1274
Remove discovered folders from status
01967db
@rk1274 rk1274 requested review from sb10 and mjkw31 April 9, 2025 14:06
mjkw31
mjkw31 approved these changes Apr 9, 2025
View reviewed changes
Copy link
Contributor

@mjkw31 mjkw31 left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@y-popov y-popov y-popov left review comments

Copilot code review Copilot Copilot left review comments

@mjkw31 mjkw31 mjkw31 approved these changes

@sb10 sb10 Awaiting requested review from sb10

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rk1274 @y-popov @sb10 @mjkw31