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

Parse address from config if not provided with '--addr' #419

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

rluetzner
Copy link
Contributor

This fixes a bug where kes would try to split and evaluate the listen address, before the config file was read. This caused kes to fail if no --addr value was provided.

With this change, the config file is parsed before we try to determine the listen address. If the --addr parameter is provided, we override the value from the config as expected.

This fixes #418 .

This fixes a bug where kes would try to split and evaluate the listen
address, before the config file was read. This caused kes to fail if no
`--addr` value was provided.

With this change, the config file is parsed before we try to determine
the listen address. If the `--addr` parameter is provided, we override
the value from the config as expected.
@rluetzner
Copy link
Contributor Author

If I remember correctly, kes previously used 7373 as it's default port. Has the default changed intentionally? If not, we should restore the default in case addr is neither defined as a CLI parameter or in a config file.

@aead
Copy link
Member

aead commented Nov 9, 2023

Dup of #416 However, LGTM 👍

@aead aead self-requested a review November 9, 2023 14:18
@aead aead merged commit 07c57a2 into minio:master Nov 9, 2023
7 checks passed
@rluetzner
Copy link
Contributor Author

If I remember correctly, kes previously used 7373 as it's default port. Has the default changed intentionally? If not, we should restore the default in case addr is neither defined as a CLI parameter or in a config file.

@aead , have you read this comment as well? Is it expected that a listen address is defined "somewhere", either as a CLI parameter or in a config file? If no, we should add something like

if (addrFlag=="") {
    addrFlag == ":7373"
}

after we tried the config.

@aead
Copy link
Member

aead commented Nov 9, 2023

I thought kesconf does set a default but apparently it does not. Also, it seems cleaner to read the config file as it is and not set any defaults at this layer. So, agree that the server should set the addr to 0.0.0.0:7373 if neither a CLI argument nor a config file value is present. 👍

@rluetzner
Copy link
Contributor Author

Perfect. I see you've added #420 already. 🙂

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.

'address' is no longer parsed from config file
2 participants