-
-
Notifications
You must be signed in to change notification settings - Fork 257
New Feature: Creating a default Variables JSON from environment variables #469
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
Conversation
Thanks, this looks really helpful. ...however, I'm not aware of The changes also seem to include #468. Can you revert those parts so each can be reviewed separately? |
Ah, apologies; reverted. Here's some relevant links. Note that the Creator Docs highlight this feature in their stable version documentation, but their example uses the @minecraft/server-admin module, which is still in beta. Looking at their dependencies, it doesn't look like I have to enable beta-APIs for this, I think we just have to run it on a standalone server (it doesn't work when testing in singleplayer). Variables & Secrets - Minecraft Creator Docs |
README.md
Outdated
@@ -191,6 +192,24 @@ There are various tools to look XUIDs up online and they are also printed to the | |||
-e ALLOW_LIST_USERS="player1:1234567890,player2:0987654321" | |||
``` | |||
|
|||
## Variables | |||
|
|||
Custom server variables are passed in just like the allowlist or as a full JSON string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to somehow include here the link to the Microsoft docs you mentioned in your reply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do.
README.md
Outdated
|
||
```shell | ||
# passing in simple expressions | ||
-e VARIABLES="var1:customStringValue,var2:1234,var3:true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like in non-json form it would be more intuitive to use equal signs such as
var1=value,var2=value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to diverge from the format for the allowlist variable? I only kept using colons to follow the previous input formatting. not an issue to change if we're okay with different formats for different env var inputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's ok since your scenario is a definitive name=value whereas the allowlist is a pairing of values due to the XUID weirdness.
dba2e24
to
b8ac785
Compare
New feature: creation of a config/default/variables.json file that uses the same input syntax as the ALLOW_LIST_USERS env var. Uses the fromjson jq cmd to help parse datatypes. No support for module-specific variables files.
b8ac785
to
93bd4a4
Compare
Sorry for the delays. Here's the pushes with the equals sign and docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for contributing this.
There are times where having server-specific variables set using environment variables is useful when deploying in Docker or Kubernetes.
This adds the creation of the variables.json in
config/default/variables.json
as an optional environment variable that can be configured during server build.