Complete the JSON schemas. #4594
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
force-pushed
the
guilt/formal-json
branch
2 times, most recently
from
61f4034
to
6637b11
Compare
vincenzopalazzo
approved these changes
Jun 11, 2021
There was a problem hiding this comment.
Look really good Rusty.
ack 6637b11
This test error proof that the JSON schema works, right? :-)
JSON report written to: report.json (69796936 bytes)
===Flaky Test Report===
test_htlc_accepted_hook_direct_restart failed (2 runs remaining out of 3).
<class 'jsonschema.exceptions.ValidationError'>
Additional properties are not allowed ('status' was unexpected)
|
Yep! And I fixed the schema! (Which updated the documentation!) |
rustyrussell
force-pushed
the
guilt/formal-json
branch
2 times, most recently
from
912bc45
to
2823b27
Compare
In general, it's better to omit a field than put in a 'null', and putting variable-named fields in an object is also a bad idea. This is reflected in how hard this is to express in JSON schema, too. Others: 1. Remove the obsolete "funding": "LOCAL" from unopened channels, but add "opener": "local" as used in normal channels. 2. htlc cltv_expiry is a u16. Signed-off-by: Rusty Russell <[email protected]> Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `funding_allocation_msat` and `funding_msat`: use `funding`. Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `last_tx_fee`: use `last_tx_fee_msat`. Changelog-Deprecated: JSON-RPC: `listfunds` `channels` `closer` is now omitted if it does not apply, not JSON `null`.
That's a terrible, terrible idea. (Documentation comes in later patch which has the schema). Also, blockheight is a u32, so simplify. Signed-off-by: Rusty Russell <[email protected]> Changelog-Deprecated: JSON-RPC: `listtransactions` `outputs` `satoshis` field (use `msat` instead).
Makes it possible to write a decent JSON schema, but means we need to carry additional data, so we create a `struct plugin_command`. We remove the unused struct dynamic_plugin, too. Signed-off-by: Rusty Russell <[email protected]>
If we have conditional fields, we often set `additionalProperties` `true` at the top-level, but then we have to make sure to cover all the possible combinations in conditionals, so unintended additions don't appear. This revealed missing fields in delinvoice, for example. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
And a new "u8" type. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
… lists. 1. listpeers has a deprecated `"closer": null`, which we need to handle in the schema, while trying not to damage our documentation too much. 2. Don't print a condition if there are no fields to print. 3. Allow a special "untyped" marker for multifundchannel which returns arbitrary JSON in a field. 4. Allow a single field return (for 'stop'). Signed-off-by: Rusty Russell <[email protected]>
This is the most complex one. It gets its own commit. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]> Changelog-Fixed: doc: Epic documentation rewrite: each now lists complete and accurate JSON output, tested against testsuite.
1. Hide the now-deprecated enable-autotor-v2-mode option. 2. Really don't print dev- options. 3. Don't print true and false as strings in some cases. Signed-off-by: Rusty Russell <[email protected]> Changelog-Fixed: JSON-RPC: `listconfigs` would list some boolean options as strings `"true"` or `"false"` instead of using JSON booleans.
This made me document what are options are now in plugins, too. Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/formal-json
branch
from
2823b27
to
44d53d9
Compare
|
Trivial rebase now #4585 is merged. |
cdecker
approved these changes
Jun 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #4585, since that cleans up JSON outputs.
This has been a mammoth effort, but it finally works. So much documentation cleanups, and JSON output fixes in here.
Glad it's finally done!
(If someone wants to write schemas & auto-gen docs for the rpc hooks, go for it!)