Skip to content

fix(entrypoint): environment variables#307

Merged
jcs47 merged 3 commits intomainfrom
fix/entrypoint_args
Jul 21, 2022
Merged

fix(entrypoint): environment variables#307
jcs47 merged 3 commits intomainfrom
fix/entrypoint_args

Conversation

@jcs47
Copy link
Copy Markdown
Contributor

@jcs47 jcs47 commented Jul 21, 2022

  • only MNEMONIC_CMD env var would be used by the entrypoint when starting tofnd
  • add support for --address and --port in the entrypoint via env vars ADDRESS and PORT


# add '--no-password' and '--unsafe' flags to args if enabled
ARGS=${NOPASSWORD:+"${ARGS} --no-password"}
ARGS=${NOPASSWORD:+"--no-password"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why remove ARG? You're modifying the args env var so need to retain all of them. Or update the individual env var, and makes ARGS the concatenation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I add to remove ARGs because the arguments weren't getting concatenated at all. Suppose ADDRESS is set but PORT was not set. Everything concatenated up to ADDRESS would be there, but then ARGS would be replaced by an empty value. Instead, I am using the += operator for assigning the returned value, which fixes the issue.

Copy link
Copy Markdown
Contributor

@sdaveas sdaveas Jul 21, 2022

Choose a reason for hiding this comment

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

consider removing lines 60-61 since ARGS is initialized here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done!

Copy link
Copy Markdown
Contributor

@sdaveas sdaveas left a comment

Choose a reason for hiding this comment

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

Thank you Joao! LGTM

@jcs47 jcs47 merged commit 4677db9 into main Jul 21, 2022
@jcs47 jcs47 deleted the fix/entrypoint_args branch July 21, 2022 20:29
talalashraf pushed a commit that referenced this pull request Jul 20, 2023
talalashraf pushed a commit that referenced this pull request Jul 20, 2023
Co-authored-by: jcs47 <11947034+jcs47@users.noreply.github.com>
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.

3 participants