Suppress curl progress meter output

[williamtrelawny]

Since I see the "Port", "BedrockPort", and "BackupCount" variables are already set to their default values, I commented these out since they are redundant. I left a brief note for users on this, like you did for the other commented-out values.

One value I was unable to verify was set to a default (likely it is not) is the TZ variable because I was unable to find where it was actually being used in the entire codebase. Could you explain to me how/where it is used and what would be its default value if left unset?

I see you have it set to America/Denver in each Dockerfile, so if commented out in docker-compose it would take this value, I understand. But what would happen if you didn't set it in the Dockerfiles either?

Signed-off-by: William Trelawny 22324745+willman42@users.noreply.github.com

[williamtrelawny]

Sorry for the confusion- I'm still getting the hang of contributing to others' projects. I accidentally overwrote my initial commit in this PR with a new commit I was trying to make a new PR for. And instead of undoing everything I just changed the name of this PR to reflect the new commit. There is 0 overlap between the two commits so there should be no issue.

Once I figure out how to make a new commit and a new PR against your current HEAD, I'll resubmit the original commit in a new PR.

[TheRemote]

Hey William,

The TZ environment variable is used by Docker and Linux basically. No special code is required on my part to take advantage of this feature. If you don't set it it will basically be the default timezone (I want to say GMT). You can read more about this here: https://dev.to/0xbf/set-timezone-in-your-docker-image-d22

I should be able to take care of adding a switch to suppress the curl output if you want cleaner logs. I'd rather not just silence everything and have zero debugging output when people have problems. I wouldn't even necessarily want this to be the default to be honest as it's really helpful for people setting things up for the first time to see what it's doing. I 100% agree there should be an option to do this added though.

So when you talk about the files set in the default Dockerfiles and removing them how are you talking about running it? I assume if you are building the container you are running your own local copy of it. Those variables are there for that reason if you want to customize it and have it baked into the container without having to use any configuration when you run it with docker run -it for example.

In the default configuration these do nothing. As you said they are set to the defaults. That doesn't mean they aren't there to serve a purpose though. Their purpose is for people to customize the container and be able to change the default baked in values without having to use docker-compose.yml, docker run -it with any parameters, things like that basically.

Removing these would just make it unclear that it's even possible to configure those via Dockerfile (it absolutely is). It would also make it more difficult for anyone who wants to do it as they will have to readd everything basically (instead of just the few missing variables that don't have a baked in image default).

The Dockerfile is only used when you build the image basically. Removing these configurable options has no benefit. It won't speed it up as everything is baked into the image at compile time. These just become default configuration values is all (and you can add the ones that are missing to configure them via Dockerfile in the same way).

In fact you can add all of the other variables I don't have baked in defaults for right into the Dockerfile as well and run it with zero parameters or configuration files essentially.

Hopefully that clarifies some things, I'll definitely get a switch added to suppress the curl output!

[williamtrelawny]

Ok makes sense. Yeah I forgot people don't always use docker compose, so yeah- leaving those env vars set in the Dockerfiles is the right thing to do. That being said, they still can be commented out in docker-compose.yml though since they are being set in the Dockerfiles as well as start.sh which you're using to build the images. Overall, not a big deal, I get it. I saw a redundancy and was just feeling froggy enough to say something about it :)

Re: the curl switch - So I'm not proposing you silence all debug output (like the -s option does), simply the progress meter. If the download fails it'll still show that it failed and why it failed. See the curl manpage on this option:

--no-progress-meter
Option to switch off the progress meter output without
muting or otherwise affecting warning and informational
messages like -s, --silent does.

[TheRemote]

Hey William,

No worries at all! I'm actually implementing it like this:

if [ -z "$QuietCurl" ]; then
    QuietCurl=""
else
    QuietCurl="--no-progress-meter"
fi

curl $QuietCurl -H "Accept-Encoding: identity" -H "Accept-Language: en" -L -A "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4.212 Safari/537.36" -o /minecraft/paperclip.jar "https://papermc.io/api/v2/projects/paper/versions/$Version/builds/$Build/downloads/paper-$Version-$Build.jar"

Basically what this does is that if the "QuietCurl" option is empty (thusly named because the point of it is to make curl more quiet in the logs) it will set this variable to nothing and do nothing. If the variable is set to anything (the intention is yes but if it's not empty it will activate) it will become --no-progress-meter from your pull request if that makes sense.

I'm just about finished with this and just need to test it. I think this is worth adding for sure!

[TheRemote]

This is finished! It actually ended up more similar to your suggestions than my original plan which was causing problems.

You should be able to pull the latest with: docker pull 05jchambers/legendary-minecraft-geyser-floodgate:latest

Or if you are building yourself you can pull the changes from here. Thanks a ton for suggesting this!