docker-minecraft-server icon indicating copy to clipboard operation
docker-minecraft-server copied to clipboard

Skip Modpack download if SKIP_GENERIC_PACK_UPDATE_CHECK is set true

Open SgtMate opened this issue 1 year ago • 5 comments

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.

SgtMate avatar Jan 06 '25 22:01 SgtMate

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

itzg avatar Jan 07 '25 00:01 itzg

@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.

SgtMate avatar Jan 07 '25 08:01 SgtMate

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.

Fronti avatar Jan 07 '25 09:01 Fronti

Download uses a behavior brought over from curl using

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

itzg avatar Jan 07 '25 13:01 itzg

@itzg Thanks for clarifing that!

SgtMate avatar Jan 07 '25 14:01 SgtMate