Skip to content
This repository has been archived by the owner on Jun 23, 2024. It is now read-only.

Commit

Permalink
Fix _check_folder
Browse files Browse the repository at this point in the history
  • Loading branch information
@SanmerDev
SanmerDev committed Sep 5, 2023
1 parent a65716e commit 28cefd2
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion sync/core/Check.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import shutil

from .Config import Config
from .Index import Index
from .Pull import Pull
Expand Down Expand Up @@ -36,7 +38,9 @@ def _check_folder(self, track, target_id):

if new_module_folder.exists():
msg = f"{target_id} already exists, remove the old directly"
self._log.d(f"_check_folder: [{track.id}] -> {msg}")
self._log.w(f"_check_folder: [{track.id}] -> {msg}")
shutil.rmtree(old_module_folder)

return True

old_module_folder.rename(new_module_folder)
Expand Down

5 comments on commit 28cefd2

@IzzySoft
Copy link

Choose a reason for hiding this comment

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

What if new_module_folder contains a different module? Shouldn't track.json be checked for id and update_to at least – and if they don't match, instead of removing old_module_folder a warning/error be thrown?

@SanmerDev
Copy link
Member Author

Choose a reason for hiding this comment

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

  • There should not be modules with the same id (they should be unique).
  • If there are two modules with the same id, one of them will be overwritten when generating modules.json (this should not happen).

@IzzySoft
Copy link

Choose a reason for hiding this comment

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

I fully agree. But if everything would always be as it should be, a check would not be needed 😉 It might be edge cases, but with the directory gone its contents are gone as well, so I'd tend on the safe side. That only one of the two would make it to the index is clear – but data gone is lost. There is no "registry" to ensure duplicates are ruled out, after all (at least not to my knowledge).

Those cases are probably very rare, so I have no "big issues" with that. Just thought I point it out 😉

@SanmerDev
Copy link
Member Author

Choose a reason for hiding this comment

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

Although this is a situation that is unlikely to occur, I will still make some modifications.

If the conditions are met, move it to the local folder.

@IzzySoft
Copy link

Choose a reason for hiding this comment

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

Thanks!

Btw: I've just updated to the latest code. Got quite a shock how fast sync is now, amazing! 😍 Hope one doesn't hit any Github API limits with larger repos (mine is now at 82 modules) 🙈 Great work, thanks a lot!

Please sign in to comment.