fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
530 stars 209 forks source link

fix: exclude config from default output in gateway-cli info rpc #5070

Open jrakibi opened 1 week ago

jrakibi commented 1 week ago

This PR addresses the feedback from issue #4365 by modifying the gateway-cli info command to exclude the config section from the default output.

Changes:

m1sterc001guy commented 1 week ago

This will unfortunately break backwards compatibility. To maintain backwards compatibility, you will need to change the include_config flag to be exclude_config so that by default the config is included. This way, old clients can continue to make the RPC as they did before, but new clients can add the exclude_config flag to have a more readable output.

The devimint tests will need to be updated. Then after a major release, we can invert the flag (or remove it entirely) so that its excluded by default.

jrakibi commented 1 week ago

@m1sterc001guy, done. I updated the flag from include_config to exclude_config, but it seems that the job for backwards-compatibility is still failing

m1sterc001guy commented 1 week ago

We need to get to the bottom of the backwards compatibility failures. Is there a default value for exclude_config? If there's no default value, I would assume that old CLI versions will fail since they do no understand the exclude_config flag.

dpc commented 1 week ago

BTW. Probably good idea to make this controlled via env var (optionally), so in scritpts and for backward compat it can be set once for all invocations.

elsirion commented 5 days ago

old CLI versions will fail since they do no understand the exclude_config flag.

Old clients won't send the exclude flag, so new servers need a default. New clients may send the exclude flag, but old servers should ignore it due to JSON.