[RPC] Static swagger by sabau · Pull Request #3880 · tendermint/tendermint

sabau · 2019-08-05T16:24:02Z

Describing statically the RPC implementation with swagger

The initial goal was to introduce proper annotations:

I tried swaggo but I hit a limitation in the usage of aliases https://github.com/sabau/tendermint/tree/2880-swagger-rpc-docs
I tried go-swagger but they don't support go modules yet, even though go-swagger itself is using them https://github.com/sabau/tendermint/tree/2880-go-swagger-rpc

With both, workarounds to make it work are possible but may create a brittle solution that will soon need to be refactored once go modules are supported (or aliases in the case of swaggo)

The first part of the definitions is organized, the second half is left as it will look once auto-generated, so we can see the differences

Referenced an issue explaining the need for the change
Updated all relevant documentation in docs
Updated all code comments where relevant
Wrote tests
Updated CHANGELOG_PENDING.md

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

codecov-io · 2019-08-05T16:33:26Z

Codecov Report

Merging #3880 into master will increase coverage by 1.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3880      +/-   ##
==========================================
+ Coverage   65.65%   66.88%   +1.23%     
==========================================
  Files         217      219       +2     
  Lines       18198    18205       +7     
==========================================
+ Hits        11948    12177     +229     
+ Misses       5382     5139     -243     
- Partials      868      889      +21

Impacted Files	Coverage Δ
privval/signer_server.go	`95.65% <0%> (-4.35%)`	⬇️
consensus/ticker.go	`91.66% <0%> (-4.17%)`	⬇️
consensus/metrics.go	`15.17% <0%> (-1.83%)`	⬇️
blockchain/v0/pool.go	`80% <0%> (-0.99%)`	⬇️
consensus/types/round_state.go
rpc/lib/client/http_client.go
consensus/types/height_vote_set.go
consensus/types/peer_round_state.go
consensus/types/codec.go
rpc/lib/client/ws_client.go
... and 12 more

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

safareli · 2019-08-06T17:35:12Z

This is definition of ResponseInfo message from proto file but it's

tendermint/abci/types/types.proto

Lines 137 to 145 in 62f97a6

    
           message ResponseInfo { 
        
             string data = 1; 
        
             string version = 2; 
        
             uint64 app_version = 3; 
        
             int64 last_block_height = 4; 
        
             bytes last_block_app_hash = 5; 
        
           }

But in swagger file you have:

tendermint/docs/spec/rpc/swagger.yaml

Lines 2115 to 2118 in a9ae00b

    
           required: 
        
             - "data" 
        
             - "last_block_height" 
        
             - "last_block_app_hash"

i.e only 3 fields are specified vs 5.

also general issue of having omitempty everywhere in generated types means non of the fileds of generated types are actually required #3882

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau · 2019-08-08T16:39:15Z

Thanks for cathing this @safareli! I will adapt the swagger and double-check what else could have been falling under this category.

tac0turtle · 2019-08-09T09:41:22Z

@mircea-c can we have this in place of https://tendermint.com/rpc/#introduction, when its merged?

sabau · 2019-08-09T12:22:44Z

@mircea-c @marbar3778 To get this working we should deploy just once the index.html I added in the same folder of the swagger file (if it's an S3 bucket let's just be sure they are both reachable permission-wise) and every time we want to update the RPC docs, just replace swagger.yaml with the new version of it

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

That's cleaner, thanks! Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

networks/local/localnode/Dockerfile

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

…lumes Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

fadeev · 2019-08-12T15:45:40Z

@marbar3778 @sabau you guys need to loop in the design team and check how the website is currently set up and what it would take to integrate this into it.

If I understand correctly, to switch to Swagger (from Slate) we would have to edit the Makefile:

https://github.com/tendermint/tendermint.com/blob/master/Makefile#L48

so that it would copy docs/spec/rpc/swagger.yaml and docs/spec/rpc/index.html from this repo into dist/rpc/ in tendermint.com repo. This shouldn't affect the tendermint.com website proper.

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau · 2019-08-13T11:10:52Z

A follow-up PR could make the network a bit more interesting to tests, adding addresses, transactions hashes to look at etc

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

thanethomson

So happy to see contract tests! 🎉 🕺

thanethomson · 2019-08-14T16:20:56Z

docs/spec/rpc/swagger.yaml

@@ -0,0 +1,2680 @@
+swagger: "2.0"
+info:
+  version: "0.32.1"


Will we have to manually update the version each time?

thanethomson · 2019-08-14T16:23:11Z

docs/spec/rpc/swagger.yaml

+    description: Evidence APIs
+schemes:
+  - https
+host: stargate.cosmos.network:26657


Should we be using this address here? Due to the more generic nature of Tendermint, I'd imagine we'd need to provide an address that's relevant to a broader user base. The default for Tendermint configuration at the moment is to bind to 127.0.0.1, so should we perhaps not just use:

Suggested change

host: stargate.cosmos.network:26657

host: localhost:26657

I do believe this is so when a user comes to the swagger docs they can hit the endpoint and get results back instead of having to spin a local node to be able to test it. similar to the try it out button here: https://cosmos.network/rpc/#/ICS0/get_node_info

Perhaps we should look into setting up a Tendermint/kvstore node here instead then, running the latest version of Tendermint? Something generic where users can follow along in the Tendermint documentation to interact with the server.

thanethomson · 2019-08-14T16:23:49Z

docs/spec/rpc/swagger.yaml

+  - name: Evidence
+    description: Evidence APIs
+schemes:
+  - https


By default we expose an HTTP interface - could we add that in here?

thanethomson · 2019-08-14T16:37:27Z

docs/spec/rpc/swagger.yaml

+          - "hash"
+        properties:
+          code:
+            type: "string"


Right now this is actually an integer. See #3898 - I'm sure we're going to rather tend towards representing all integers as integers instead of strings in future.

I've tried adding this comment twice now and GitHub seems to consistently get the line number wrong somehow in the preview 🤷‍♂. Just in case, I'm referring to docs/spec/rpc/swagger.yaml#2666.

melekes · 2019-08-16T08:53:44Z

.circleci/config.yml

+  contract_tests:
+    working_directory: /home/circleci/.go_workspace/src/github.com/tendermint/tendermint
+    machine:
+      image: circleci/classic:latest


why not default one or nodejs?

For this @sabau mentioned its either start in node and download go or start in go and download node, he chose node.

he chose neither!!

melekes · 2019-08-16T08:54:02Z

.circleci/config.yml

+    steps:
+      - checkout
+      - run:
+          name: Test RPC endpoints agsainst swagger documentation


Suggested change

name: Test RPC endpoints agsainst swagger documentation

name: Test RPC endpoints against swagger documentation

melekes · 2019-08-16T08:54:36Z

CONTRIBUTING.md

+### RPC Testing
+
+If you contribute to the RPC endpoints it's important to document your changes in the [Swagger file](./docs/spec/rpc/swagger.yaml)
+To test your changes you should install `nodejs` version `v11.15.*` and run:


don't think we need version here

tac0turtle

⚡️ created follow up an issue for nonaddressed comments, @sabau thank you for your contributions

safareli · 2019-08-19T14:16:50Z

When would this be hosted on tendermint website?

sabau added 12 commits August 2, 2019 17:54

manually swagging

00ed3b8

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

three definitions with polymorphism

4720ecc

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

added blockchain and block

e6a76ae

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

low quality generation, commit, block_response and validators

5fd7995

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

genesis and consensus states endpoints

caf41f8

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

fix indentation

d62f029

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

consensus parameters

4f42d78

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

fix indentation

12dae84

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

add height to consensus parameters endpoint

501ff28

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

unconfirmed_txs and num_unconfirmed_txs

33bbccc

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

add missing query parameter

4c9db2a

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

add ABCI queries

a9ae00b

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau changed the title ~~2880 static swagger~~ [RPC] Static swagger Aug 5, 2019

sabau added 2 commits August 6, 2019 18:17

added index document for swagger documentation

037b9f2

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

Merge branch 'master' into 2880-static-swagger

47ecd0e

safareli mentioned this pull request Aug 6, 2019

Generate full tendermint client using swagger.json f-o-a-m/kepler#47

Closed

add missing routes

b23b168

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau marked this pull request as ready for review August 8, 2019 16:39

sabau requested review from ebuchman, melekes and xla as code owners August 8, 2019 16:39

sabau added 4 commits August 9, 2019 18:43

contract tests added on CCI

69d06f4

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

contract tests job should be in the test suite

ddd06fb

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

simplify requirements to test

06db9e8

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

typo

e4f9fb0

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

melekes and others added 3 commits August 12, 2019 14:43

Merge branch 'master' into 2880-static-swagger

86c62de

Update scripts/get_tools.sh

f8bba32

That's cleaner, thanks! Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>

xz not supported by cci image, let's keep it simple

94bc7a3

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

tac0turtle mentioned this pull request Aug 12, 2019

rpc: landing page incorrectly reports all endpoints as needing args #3618

Closed

REMOVE-indirect debug of CCI paths

de8ed0c

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

safareli mentioned this pull request Aug 12, 2019

Remove JSON-RPC wrapping when URI over HTTP is used? #3895

Closed

dirty experiment, volume is empty but binary has been produced

5b858ab

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

melekes reviewed Aug 12, 2019

View reviewed changes

networks/local/localnode/Dockerfile Outdated Show resolved Hide resolved

sabau added 3 commits August 12, 2019 17:09

dirty experiment, volume is empty but binary has been produced

7768c2d

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

dirty experiment going on

062f18a

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

locally works, CCI have difficulties with second layaer containers vo…

cc7ecc6

…lumes Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau added 4 commits August 13, 2019 12:17

restore experiment, use machine instead of docker for contract tests

98924bc

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

simplify a bit

eae5372

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

rollback on machine golang

4822e23

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

Document the changes

4f943fe

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

sabau added 3 commits August 13, 2019 13:57

Merge branch 'master' into 2880-static-swagger

82bab0b

Changelog

6aed391

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

comments

8fb4f9e

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>

thanethomson reviewed Aug 14, 2019

View reviewed changes

Merge branch 'master' into 2880-static-swagger

9a90fe0

melekes reviewed Aug 16, 2019

View reviewed changes

tac0turtle mentioned this pull request Aug 16, 2019

Swagger Static Follow-up #3903

Closed

6 tasks

Merge branch 'master' into 2880-static-swagger

652fc4f

tac0turtle approved these changes Aug 16, 2019

View reviewed changes

tac0turtle merged commit ead3eef into tendermint:master Aug 16, 2019

safareli mentioned this pull request Aug 19, 2019

docs: switch to Swagger for RPC #2419

Closed

	name: Test RPC endpoints agsainst swagger documentation
	name: Test RPC endpoints against swagger documentation

Conversation

sabau commented Aug 5, 2019 • edited by thanethomson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

safareli commented Aug 6, 2019

Uh oh!

sabau commented Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tac0turtle commented Aug 9, 2019

Uh oh!

sabau commented Aug 9, 2019

Uh oh!

Uh oh!

fadeev commented Aug 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sabau commented Aug 13, 2019

Uh oh!

thanethomson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thanethomson Aug 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tac0turtle left a comment

Choose a reason for hiding this comment

Uh oh!

safareli commented Aug 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sabau commented Aug 5, 2019 •

edited by thanethomson

Loading

codecov-io commented Aug 5, 2019 •

edited

Loading

sabau commented Aug 8, 2019 •

edited

Loading

fadeev commented Aug 12, 2019 •

edited

Loading

thanethomson Aug 14, 2019 •

edited

Loading