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

Consider using defaults for missing properties in config files #1490

Open
bluekeyes opened this issue Dec 12, 2019 · 4 comments
Open

Consider using defaults for missing properties in config files #1490

bluekeyes opened this issue Dec 12, 2019 · 4 comments

Comments

@bluekeyes
Copy link

Is your feature request related to a problem? Please describe.

The documentation suggested to me that configuration properties that are not set in a configuration file will use the documented defaults; this is the behavior if environment variables are used for configuration. Instead, any properties not explicitly defined in the configuration file use the zero values for their types.

Describe the solution you'd like

The server should use default values for any properties that are not set in the configuration file. This is how it already works when you do not provide a configuration files and use environment variables.

Describe alternatives you've considered

The documentation could be updated to clarify that users should copy the sample configuration and change values, rather than using it as a reference and writing their own file. This may already be mentioned somewhere, but I was unable to find it.

Additional context

This issue is split out of #1336, which has some additional information.

@arschles
Copy link
Member

@bluekeyes sorry I haven't gotten back to you in so long. My travel schedule has calmed down significantly so I hope to be able to have an ongoing discussion.

I'll get #1492 into a better state so we can get logs for DownloadMode into the next release. Also, from #1336, it sounds like the next step is to mark fields as required. Would you be up for submitting a PR with required tags on some of the config fields that you've run into?

@arschles
Copy link
Member

@bluekeyes I haven't forgotten about this. still working on #1492. When that's merged, I'll have a good pattern to work on, to expand validation to other config fields

@bluekeyes
Copy link
Author

Thanks for the continued work on this. I'm pretty sure #1492 will fix the problem I ran into with DownloadMode, but I don't remember if there were other "required, but not validated" properties.

I don't have enough familiarity with Athens to know offhand what other properties need to be required, and I don't know if I'll have time to go through code to figure it out. But I might be able to submit a PR to set defaults for unset properties if that's desirable behavior.

@arschles
Copy link
Member

@bluekeyes you got it. if it makes Athens easier to run, I'm in 😄

If you'd like to submit PRs for setting required/default please feel free. I'm going to be looking at our config code alongside our getting started docs sometime soon as well.

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

No branches or pull requests

2 participants