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

Extend the Properties api to better support nested configuration #3944

Closed
soulomoon opened this issue Jan 12, 2024 · 2 comments · Fixed by #3947 or #3952
Closed

Extend the Properties api to better support nested configuration #3944

soulomoon opened this issue Jan 12, 2024 · 2 comments · Fixed by #3947 or #3952

Comments

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 12, 2024

          > I don't know whether we might want more configuration options later, but perhaps it would be wise to nest all these in config.tokenMapping or something?

I think this one would be better. I am going to extend the properties api to better support nested configuration.

Originally posted by @soulomoon in #3940 (comment)

Since the plugin api's support for nested configuration is poor. It would be better to extend it to enable a type safe configuration for nested configuration.

Steps

Towards this goal there are two steps of implementation.

  1. The current implementation use (unsafecoerce), and the safty is provided by hand obeying the rules in property map.
    Which is not really safe. We replace it with safer implementation. doing in Properties API: Remove unsafe coerce in favor of type class based method in  #3947
  2. Extend the configuration api under the above safer implementation to provide nicer nested configuration. doing in 3944 extend the properties api to better support nested configuration #3952

It should support #3937 better

@michaelpj
Copy link
Collaborator

FWIW I think this is probably not the most important thing to do. We have generally coped with one layer of custom plugin config, and if you need more you can fake it by adding prefixes/suffixes to option names. Not lovely, but not worth a huge amount of effort to fix, IMO.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 13, 2024

No worries. Things seems to be going smoother than I imagin, It is very close to be done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment