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

Config parameters that have defaults should be optional #105

Closed
hrj opened this issue Dec 13, 2021 · 6 comments · Fixed by #122
Closed

Config parameters that have defaults should be optional #105

hrj opened this issue Dec 13, 2021 · 6 comments · Fixed by #122

Comments

@hrj
Copy link
Contributor

hrj commented Dec 13, 2021

When a config parameter has a default value, the user should be able to optionally omit it from the config file.

For example, since playgroundEnabled has a default value of true, the user need not specify it.

Currently, the server throws an exception for missing parameters.

@hrj hrj added this to the version 1.2 milestone Dec 13, 2021
@rr83019
Copy link
Contributor

rr83019 commented Jan 7, 2022

Here's a list of config parameters, please choose the ones we want to make optional:

  • port
  • address
  • throttle
  • seed
  • captchaExpiryLimit
  • threadDelay
  • playgroundEnabled
  • corsHeader
  • maxAttempts

@hrj
Copy link
Contributor Author

hrj commented Jan 7, 2022

Looks like all of them can be optional, and have sane defaults 😄

@rr83019
Copy link
Contributor

rr83019 commented Jan 7, 2022

Cool, any suggestion on what the defaults should be?

@hrj
Copy link
Contributor Author

hrj commented Jan 7, 2022

The defaults can be what is inside the automatically created config file:

{
  "randomSeed" : some random integer,
  "port" : 8888,
  "address" : "0.0.0.0",
  "captchaExpiryTimeLimit" : 5,
  "throttle" : 1000,
  "threadDelay" : 2,
  "playgroundEnabled" : true,
  "corsHeader" : "",
  "maxAttempts" : 10,
}

@rr83019
Copy link
Contributor

rr83019 commented Jan 7, 2022

Making the playgroundEnabled field string from bool.

The extract function of json4s is not able to convert JBool to java.lang.String

@hrj
Copy link
Contributor Author

hrj commented Jan 8, 2022

Ideally, we shouldn't change the type because it will break existing configurations.

I have added a suggestion to the PR, please have a look.

@hrj hrj closed this as completed in #122 Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants