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

431 Add environment variable options to Spacewalk #483

Merged
merged 8 commits into from
Feb 12, 2024

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Jan 29, 2024

closes #431

adding the ff. arguments an ENV option:

  1. KEYRING
  2. KEYFILE
  3. KEYNAME
  4. SPACEWALK_PARACHAIN_URL
  5. STELLAR_VAULT_SECRET_KEY_FILEPATH
  6. STELLAR_OVERLAY_CONFIG_FILEPATH
  7. AUTO_REGISTER

note for AUTO_REGISTER: since the value must be a tuple, make sure to enclose it with double quotes.

🙅 ❌ export AUTO_REGISTER=0,GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN:USDC,1000000 ❌ 🙅

✅ 👌 export AUTO_REGISTER="0,GAKNDFRRWA3RPWNLTI3G4EBSD3RGNZZOY5WKWYMQ6CQTG3KIEKPYWAYC:USDC,1000000" 👌 ✅

For arguments that accept option, make sure to enclose it with double quotes with the ACTUAL value.
🙅 ❌ export MAX_NOTIFS_PER_SUBSCRIPTION="1" ❌ 🙅
🙅 ❌ export MAX_NOTIFS_PER_SUBSCRIPTION="Some(1)" ❌ 🙅

✅ 👌 export MAX_NOTIFS_PER_SUBSCRIPTION=1 👌 ✅

@b-yap b-yap requested a review from a team January 29, 2024 11:45
@b-yap b-yap force-pushed the 431-add-environment-variable-options-to-spacewalk branch from eb6310d to f5ef6db Compare January 30, 2024 05:40
Copy link
Contributor

@TorstenStueber TorstenStueber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but is there a reason not to make all configuration parameters available as environment variables? We don't really use the ones that you didn't add but they could be useful for other users.

@b-yap
Copy link
Contributor Author

b-yap commented Feb 7, 2024

@ebma I added env option for ALL args.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I guess there is no way to spit out all available names of environment variables from the vault binary, I think it's best to give each env variable the very same name as the config item. Then it's at least somewhat obvious which names are used/available.

@b-yap b-yap merged commit 35a4895 into main Feb 12, 2024
2 checks passed
@b-yap b-yap deleted the 431-add-environment-variable-options-to-spacewalk branch February 12, 2024 22:24
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.

Add environment variable options to Spacewalk
3 participants