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

Clean up Config Field Naming #5179

Closed
fm3 opened this issue Feb 18, 2021 · 2 comments · Fixed by #5208
Closed

Clean up Config Field Naming #5179

fm3 opened this issue Feb 18, 2021 · 2 comments · Fixed by #5208

Comments

@fm3
Copy link
Member

fm3 commented Feb 18, 2021

The application.conf keys are messy.

  • braingames.binary is not a semantic name anymore
  • There is no consistent handling of units (I’d say they should be in the names, except for durations, where the Duration/FiniteDuration type should be used instead of integers)
  • uri vs url vs address + port should be unified (not sure yet which is best)
  • agglomerates should get their own block rather than use prefix to group
  • Unclear what is in webKnossos block and what is outside of it
  • naming: allowX vs xAllowed
  • allow vs enable
  • Check which config fields should really be exposed (e.g. does silhouette need that many?)
  • Move silhouette to application.authentication ?
  • get rid of features block; instead annotate in wkConf.scala which fields should be exposed.

Problem is that this would require extensive changes for operators in how they overwrite the keys, so we should consider if this is really worth it, and write a good explanation for the migrations guide

@normanrz
Copy link
Member

Would it be possible to write an automated migration or support the keys in a backwards-compatible manner?

@fm3
Copy link
Member Author

fm3 commented Feb 23, 2021

Possible, yes, but not trivial. As discussed in person, I think a complete and concise description in the migration guide will suffice. This is what we did with previous config changes too. If anyone is having trouble, we can probably help them as well.

@fm3 fm3 mentioned this issue Mar 3, 2021
14 tasks
@fm3 fm3 closed this as completed in #5208 May 3, 2021
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