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

Torrents! #40

Merged
merged 4 commits into from
Aug 7, 2024
Merged

Torrents! #40

merged 4 commits into from
Aug 7, 2024

Conversation

jojo2357
Copy link
Owner

@jojo2357 jojo2357 commented Aug 7, 2024

closes #35

Changelog:

  • Fix index corrupted bug
  • add torrenting instead of forcing http update
  • Fix octal update

If the newest zim and torrent both existed, would give strange message
Dont do that
@Jaifroid
Copy link
Collaborator

Jaifroid commented Aug 7, 2024

@jojo2357 I've looked through the README, and it seems well worded and accurate. I didn't see anything I would want to change. Thanks!

@jojo2357
Copy link
Owner Author

jojo2357 commented Aug 7, 2024

Does purging work as intended? Right now iirc t mode does no such thing, but the standard version does still purge. This could be a problem, or if the user is advanced enough we hope they know whether or not they want to keep old versions.

@Jaifroid
Copy link
Collaborator

Jaifroid commented Aug 7, 2024

Does purging work as intended? Right now iirc t mode does no such thing, but the standard version does still purge. This could be a problem, or if the user is advanced enough we hope they know whether or not they want to keep old versions.

-t mode doesn't understand -p (and I think that's correct, -t mode should never purge, and so -p is redundant). I'm going on having done a dry-run with and without -p and noticing that the outcome would have been the same, and different from doing the same without -t. But in terms of a "live run", I ran it with -p. I didn't quite dare run it without, even though the dry-run had indicated it would be safe! I have a lot of old ZIMs I use for testing / development on Kiwix, and I don't want to lose them. So I would strongly urge against introducing a purge function in -t mode.

@jojo2357
Copy link
Owner Author

jojo2357 commented Aug 7, 2024

works for me. if you torrent then you should remove the torrent and the zim

With @DocDrydenn 's blessing, I will hit that merge button. I will give it about a day for any other comments.

@jojo2357 jojo2357 marked this pull request as ready for review August 7, 2024 08:11
Copy link
Collaborator

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

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

I can't review the code, but I've reviewed through testing, and agree with the proposed functionality. I've also reviewed the README. Thank you for this great PR!

Copy link
Collaborator

@DocDrydenn DocDrydenn left a comment

Choose a reason for hiding this comment

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

Hello all!

Just wanted to give a quick update. My homelab has been undergoing upgrades (racking changes, storage upgrades, and a full network overhaul) for about the last two weeks... and I've been unable to physically test the scrip updates against my ZIM library.

That said, I don't let me be the reason this merge gets held up. I gave the changes a look and didn't see anything that looks to be a problem. Knowing that you all have tested, I say Go.

@jojo2357 jojo2357 merged commit 596425b into main Aug 7, 2024
@jojo2357
Copy link
Owner Author

jojo2357 commented Aug 7, 2024

Well we have the -d flag for a reason!

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

Successfully merging this pull request may close these issues.

Torrent feature
3 participants