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

Skip Modpack download if SKIP_GENERIC_PACK_UPDATE_CHECK is set true #3239

Open
SgtMate opened this issue Jan 6, 2025 · 5 comments
Open

Skip Modpack download if SKIP_GENERIC_PACK_UPDATE_CHECK is set true #3239

SgtMate opened this issue Jan 6, 2025 · 5 comments

Comments

@SgtMate
Copy link
Contributor

SgtMate commented Jan 6, 2025

Enhancement Type

Improve an existing feature

Describe the enhancement

I have noticed, that if you enable SKIP_GENERIC_PACK_UPDATE_CHECK, the pack gets downloaded anyway.
This was contradictory to my expectations as it states in the documentation:

If applying large generic packs, the update can be time-consuming. To skip the update set SKIP_GENERIC_PACK_UPDATE_CHECK to "true". Conversely, the generic pack(s) can be forced to be applied by setting FORCE_GENERIC_PACK_UPDATE to "true".

To me that means that no request is made to the modpacks download server, but my testing showed that a download is done at first, but it seems that nothing is happening to the downloaded files.

This might be related to what is happening here

I think the download should be skipped completely if the flag is set, as this might cause many unneded requests to the download servers of the custom modpacks (many of them hosted by volunteers) due to this variable being set in some of the example compose files in the expectation that it will stop download requests to the modpacks filehosting servers.

@itzg
Copy link
Owner

itzg commented Jan 7, 2025

I can understand the confusion. To give some context, some users had such large generic pack files that the download was fast or the file was local, but the unzipping and checksum calculation was quite lengthy. The unzipping and checksum is what is referred to as the "update" step and hence the variable named SKIP_GENERIC_PACK_UPDATE_CHECK.

What's adding to the confusion is that there is a log statement "Downloading generic pack ..." that logs regardless of if the download could be skipped. The download will skip if the file was already present and the file server doesn't report a newer copy is available.

While I don't want to change the meaning of an "update", which again has nothing to do with the download step, I can see a couple of things to improve:

  • I'll remove the unconditional "Downloading" log since that is not really helpful
  • I'll enable an option on the download command that will log after each file is downloaded. Nothing is logged when the file didn't need to be downloaded
  • I'll introduce a variable called GENERIC_PACK_SKIP_DOWNLOAD_EXISTS that will skip the download entirely if the file exists and won't query the file server to see if a newer file is available

@SgtMate
Copy link
Contributor Author

SgtMate commented Jan 7, 2025

@itzg Thank you for the quick reply on my issue and thank you for the explanation. I now understand the term update in this context.
I like your proposed changes and think that this will help improve the overall experience of using generic packs and also lessen the strain on smaller mod projects that dont rely on the big hosting services.

One question I have left is how the container is checking for a newer version on the download server?

Also I might want to add to the improvements, that the corresponding section in the documentation should be updated to further clarify what the different variables do and dont do.

@Fronti
Copy link

Fronti commented Jan 7, 2025

The download will skip if the file was already present and the file server doesn't report a newer copy is available.

At least in my testing it does fully re-download the pack (GTNH) every time I start the container. Both ifconfig and router report the ~400 MB of incoming traffic. Maybe your check for a newer version isn't compatible with GTNH's download server? Either way the introduction of that "download once to install and never again" type of variable you mentioned would definitely solve edge-cases like this.

@itzg
Copy link
Owner

itzg commented Jan 7, 2025

Download uses a behavior brought over from curl using

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since

@SgtMate
Copy link
Contributor Author

SgtMate commented Jan 7, 2025

@itzg Thanks for clarifing that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

No branches or pull requests

3 participants