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

use string instead of bool value in docker-copose environment #267

Merged
merged 6 commits into from
Sep 11, 2024

Conversation

superbjorn09
Copy link
Contributor

services.satisfactory-server.environment.ROOTLESS contains false, which is an invalid type, it should be a string, number, or a null
services.satisfactory-server.environment.STEAMBETA contains false, which is an invalid type, it should be a string, number, or a null

wolveix and others added 5 commits September 10, 2024 16:26
services.satisfactory-server.environment.ROOTLESS contains false, which is an invalid type, it should be a string, number, or a null
@wolveix
Copy link
Owner

wolveix commented Sep 10, 2024

Are you experiencing this as an issue on your end? Docker compose has no issues with these

@superbjorn09
Copy link
Contributor Author

Its probably just a compatibility thingi, cause I use a rather old version of docker-compose:
docker-compose version 1.25.0

making this a string "should" make it work on older and newer version, but you better double check if the params get applied correctly if you re using a newer version of docker-compose

@DenuxPlays
Copy link
Contributor

I think yaml does not known the difference between strings and bool or am I wrong?

@skupfer
Copy link

skupfer commented Sep 10, 2024

It should be a string as false is a special token in yaml, namely boolean. The application which interprets yaml will handle it. https://devops.stackexchange.com/a/542

@wolveix wolveix merged commit 02e7835 into wolveix:main Sep 11, 2024
2 checks passed
@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

Thanks!

@voruti
Copy link
Contributor

voruti commented Sep 12, 2024

This unfortunately isn't correct, you're mixing up things.

The docker-compose.yml at hand uses a list of environment variables:

    environment:
      - ROOTLESS='false'
      - STEAMBETA='false'

where the whole line is just a YAML string (everything behine the "- ").

On the contrary there also are environment variables as YAML objects/keys:

    environment:
      ROOTLESS: "false"
      STEAMBETA: "false"

where there are YAML key-value relations.

It should be a string as false is a special token in yaml, namely boolean.

This is where that applies and "false" must be put into quotes.

Generally speaking, Docker can handle both variants.
But: putting quotes around "false" while using the first variant is wrong. In that case, the quotes are picked up literally and get put into the environment variable itself.

The command docker compose config allows to see a summary of the configuration. The output of that command for the docker-compose.yml introduced by this PR is the following (excerpt):

    environment:
      ROOTLESS: '''false'''
      STEAMBETA: '''false'''

(wrongly escaped boolean strings)

Prior to the PR it was:

    environment:
      ROOTLESS: "false"
      STEAMBETA: "false"

Here Docker automatically parsed the first variant (where the whole line is a single string) and output the configuration in the format of the second variant (with correctly escaped "false").

@skupfer
Copy link

skupfer commented Sep 12, 2024

Thanks for your reply @voruti and proving my point that it does make a difference.

As I only replied to Denux on the matter I didn't check the commit properly as I have seen the key:value format here before. I think I mixed it up here 82797b5 but only the readme got adjusted. My bad.

wolveix added a commit that referenced this pull request Sep 12, 2024
@wolveix
Copy link
Owner

wolveix commented Sep 12, 2024

Thanks for the correction @voruti. That's what I thought I remembered, but I hastily merged this while dealing with other 1.0 things. It was only in the docker-compose.yml file, while most people copy from the README :) My apologies for not reviewing this properly myself

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.

7 participants