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.

Sign up for GitHub

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

Jump to bottom

Fix configuration via environment variables for optional parameters #3702

Merged
merged 3 commits into from
Jun 17, 2021
Merged

Fix configuration via environment variables for optional parameters #3702

merged 3 commits into from
Jun 17, 2021

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Jun 17, 2021

Fixes #3696

This PR fixes a bug where the configuration variables listed below were ignored if they were configured via environment variables instead of command line arguments:

CAPTIVE_CORE_LOG_PATH
CAPTIVE_CORE_PEER_PORT
CAPTIVE_CORE_HTTP_PORT

@tamirms tamirms changed the title Fix bug configuration via environment variables for optional parameters Fix configuration via environment variables for optional parameters Jun 17, 2021
@tamirms tamirms force-pushed the fix-optional-config branch from 1238d5b to 0d5b84a Compare June 17, 2021 10:57
@tamirms tamirms requested a review from a team June 17, 2021 10:58
@tamirms tamirms force-pushed the fix-optional-config branch from 385853a to 2769fa9 Compare June 17, 2021 10:58
bartekn
bartekn approved these changes Jun 17, 2021
View reviewed changes
Copy link
Contributor

@bartekn bartekn left a comment

Choose a reason for hiding this comment

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

LGTM! I'm just wondering if CaptiveCoreTomlParams.HTTPPort can't be uint instead of *uint. 0 value disabled the HTTP server so should work with a non-pointer default value.

@tamirms
Copy link
Contributor Author

tamirms commented Jun 17, 2021

@bartekn if http port is omitted the default value assumed by stellar core is actually is 11626 not 0

@tamirms tamirms merged commit f3d0f04 into stellar:master Jun 17, 2021
@tamirms tamirms deleted the fix-optional-config branch June 17, 2021 13:25
@bartekn
Copy link
Contributor

bartekn commented Jun 17, 2021

@bartekn if http port is omitted the default value assumed by stellar core is actually is 11626 not 0

Correct but Horizon will set it to the default value of 0 (in flags.go).

@tamirms tamirms mentioned this pull request Jun 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartekn bartekn bartekn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CAPTIVE_CORE_LOG_PATH config value is ignored
2 participants
@tamirms @bartekn