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

Toml sections? #40

Open
dr-orlovsky opened this issue May 25, 2020 · 8 comments
Open

Toml sections? #40

dr-orlovsky opened this issue May 25, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@dr-orlovsky
Copy link

Sorry if I missed something, but looking in the docs I have not found possibility to use multiple configuration sections, like

[networking]
ip = "192.168.0.1"
port = 8000

[cli]
enable = true
@dr-orlovsky dr-orlovsky changed the title Toml subsections? Toml sections? May 25, 2020
@Kixunil
Copy link
Owner

Kixunil commented May 25, 2020

While not impossible, the biggest issue with such sections would be inconsistency with arguments. I also suspect it'd make the parser much more complex.

It's already "possible" to do it by specifying your own struct in type, which impls serde::Deserialize, but then either you have to figure out a way to specify whole section in a single argument by implementing configure_me::parse_arg::ParseArg or drop the CLI/ENV support for that argument. (I actually used the latter in btc-rpc-proxy since specifying users on command line would be clunky anyway.)

While certainly nice to have, I kinda fail to see much value in it TBH. Especially how they implemented it in LND (not Toml, but similarly looking), where it doesn't help making the keys shorter (you still have to write bitcoind.foo inside bitcoind section) and if I remember correctly, you can use meaningless names like [red_elephant]. At that point, simple comments are superior.

I'm open to being convinced by some very good reason. It obviously must be such a good reason that I'd be willing to pause my work on improving Bitcoin privacy to do it... I'm also wiling to accept a PR solving it by introducing config_section key to argument and switch specification and generating sub-structs. I think it'd be a good idea to think about if the time of the person implementing it can be spent on more important things. :)

@Kixunil Kixunil added the enhancement New feature or request label May 25, 2020
@dr-orlovsky
Copy link
Author

dr-orlovsky commented May 26, 2020

Well, with the nodes we do (bp, lnp, rgb) we have multiple daemons + cli tools per repo (i.e. multiple binaries), and need to configure each of them. Creating a special config file for each one looks strange, so my idea was to have a section per binary in the same config file...

This is related to #41 (comment) basically

@Kixunil
Copy link
Owner

Kixunil commented May 26, 2020

Hmm, this is genuinely interesting. Will need to think about it more.

Note that I personally found splitting stuff between files easier to manage. With configure_me, it's trivial to have common config for various services - just use --conf multiple times. In certain scenarios, like system daemons, including whole directory is even better. I do this in bitcoin-rpc-proxy within Cryptoanarchy Deb repo for instance, which allows me to protect different parts of the configuration (credentials can be in a file with 600, public API can be 644)

@dr-orlovsky
Copy link
Author

Ok, how I can manage multiple build scripts etc for multiple config files and multiple binaries in the same crate?

@Kixunil
Copy link
Owner

Kixunil commented May 28, 2020

Unfortunately, you currently can't because the output is written into a file with fixed name, so one would overwrite the other. Is it OK, if I address this issue by the end of weekend?

Also, it occurred to me that being able to include another config specification might be useful if you have multiple binaries with common options. Would it help you?

@dr-orlovsky
Copy link
Author

End of week will work! And yes, multiple binaries in my case do share the same options

@Kixunil
Copy link
Owner

Kixunil commented Jun 1, 2020

Sorry for it taking me a bit longer. Unfortunately, I'm more busy than expected lately, so I at least implemented multiple-binary support (thankfully it had more code prepared than what I remembered. :)).

The new released versions should unblock you now. I assume include feature is not that critical and I will need to postpone it a bit. I hope duplication is not that huge issue yet. (You could hack it by concatenating files in build script if you need it really bad.)

@dr-orlovsky
Copy link
Author

Very good, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants