Node API Documentation Server #3925

parsonsmatt · 2018-12-04T01:28:49Z

Description

This PR implements the documentation swagger server for the Node API.

TODO:

Accept config from cli

Linked issue

Type of change

🐞 Bug fix (non-breaking change which fixes an issue)
🛠 New feature (non-breaking change which adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)
🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
🔨 New or improved tests for existing code
⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

I have read the style guide document, and my code follows the code style of this project.
If my code deals with exceptions, it follows the guidelines.
I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

I have added tests to cover my changes.
All new and existing tests passed.

QA Steps

Screenshots (if available)

How to merge

Send the message bors r+ to merge this PR. For more information, see
docs/how-to/bors.md.

KtorZ

One blocker: serveImpl vs serveDocImpl, otherwise 👍

KtorZ · 2018-12-04T07:40:25Z

node/src/Cardano/Node/API/Swagger.hs

+    -> Maybe TlsParams
+    -> IO ()
+forkDocServer prxy swVersion ip port' tlsParams =
+    serveImpl


*serveDocImpl

There's a slight difference between both: we don't check nor require x509 client certificates for the doc server. Otherwise, it will require having setup those certificates in your browser in order to navigate and browse the document. Very unpractical, especially since certificates change from an environment to another.

KtorZ · 2018-12-04T07:41:17Z

node/src/Cardano/Node/API/Swagger.hs

+                & at "address" ?~ (Inline $ mempty
+                    & type_ .~ SwaggerString
+                    )
+                )


I wonder, don't we have schema for Coin and Address already 🤔 ?

KtorZ · 2018-12-04T07:46:40Z

lib/src/Pos/Util/Swagger.hs

+    <redoc spec-url="../SERVANT_SWAGGER_UI_SCHEMA"></redoc>
+    <script src="redoc.min.js"> </script>
+  </body>
+</html>|]


NOTE: upstream PR has been accepted some weeks ago and the default template has been fixed. So, using
servant-swagger-ui-redoc-0.3.2.1.22.2 should make this obsolete; using redocSchemaUIServer would be enough 😄

KtorZ · 2018-12-04T17:33:54Z

lib/src/Pos/Client/CLI/NodeOptions.hs

        <*> tlsParamsParser
        <*> debugModeParser
+        <*> addressParser "node-doc-address" (localhost, 8084)


I think we should be defaulting to 8080 (api) and 8180 (doc) as this is the current default in our wallet backend:

monitoringApiPortParser :: Parser Word16 monitoringApiPortParser = CLI.webPortOption 8080 "Port for the monitoring API."

Then there will be a clash for the case when the node and wallet are running on the same host

Well, that's okay, we won't expose the monitoring API from the node anymore!

KtorZ · 2018-12-04T17:35:15Z

node/src/Cardano/Node/API/Swagger.hs

+documentationApi curSoftwareVersion prxy = toSwagger prxy
+    & info.title   .~ "Cardano Node API"
+    & info.version .~ fromString (show curSoftwareVersion)
+    & host ?~ "127.0.0.1:8083"


🤔 Any ways to take that from the doc -- though quite anecdotal

parsonsmatt · 2018-12-04T18:24:46Z

bors r+

3925: Node API Documentation Server r=parsonsmatt a=parsonsmatt ## Description This PR implements the documentation swagger server for the Node API. TODO: - [x] Accept config from cli ## Linked issue Co-authored-by: parsonsmatt <[email protected]> Co-authored-by: Matt Parsons <[email protected]>

iohk-bors · 2018-12-04T19:10:05Z

Build failed

ci/hydra:serokell:cardano-sl:required

parsonsmatt · 2018-12-04T23:53:32Z

bors r+

3925: Node API Documentation Server r=parsonsmatt a=parsonsmatt ## Description This PR implements the documentation swagger server for the Node API. TODO: - [x] Accept config from cli ## Linked issue Co-authored-by: parsonsmatt <[email protected]> Co-authored-by: Matt Parsons <[email protected]>

iohk-bors · 2018-12-05T00:37:30Z

Build failed

ci/hydra:serokell:cardano-sl:required

parsonsmatt · 2018-12-05T01:10:39Z

bors r+

3925: Node API Documentation Server r=parsonsmatt a=parsonsmatt ## Description This PR implements the documentation swagger server for the Node API. TODO: - [x] Accept config from cli ## Linked issue Co-authored-by: parsonsmatt <[email protected]> Co-authored-by: Matt Parsons <[email protected]>

iohk-bors · 2018-12-05T01:48:26Z

Build succeeded

parsonsmatt added 7 commits December 3, 2018 10:13

Qualify imports

df32ae0

Start getting those docs together

a77c374

check

26df663

Overlapping :(

0b1a788

liberate the instance

40d27c1

pkgs

e292ea5

Take doc address from CLI

6c59ba6

parsonsmatt requested a review from KtorZ December 4, 2018 01:59

KtorZ suggested changes Dec 4, 2018

View reviewed changes

parsonsmatt added 2 commits December 4, 2018 09:12

serveDocImpl

37d130d

Merge develop

e2c25fa

KtorZ reviewed Dec 4, 2018

View reviewed changes

KtorZ approved these changes Dec 4, 2018

View reviewed changes

parsonsmatt mentioned this pull request Dec 4, 2018

Modifications to work with node api docs input-output-hk/cardano-wallet-legacy#130

Merged

7 tasks

Use the right commit

f3f85de

stylish

6952a94

parsonsmatt force-pushed the matt/node-api-docs branch from f3ebc37 to 6952a94 Compare December 4, 2018 23:48

Merge branch 'develop' into matt/node-api-docs

7de5853

stylin

31e1e91

iohk-bors bot merged commit 31e1e91 into develop Dec 5, 2018

iohk-bors bot deleted the matt/node-api-docs branch December 5, 2018 01:48

KtorZ mentioned this pull request Dec 11, 2018

Start HTTP Server & Implement API on The Node's End input-output-hk/cardano-wallet-legacy#30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node API Documentation Server #3925

Node API Documentation Server #3925

parsonsmatt commented Dec 4, 2018 •

edited

Loading

KtorZ left a comment

KtorZ Dec 4, 2018

KtorZ Dec 4, 2018

KtorZ Dec 4, 2018

KtorZ Dec 4, 2018

parsonsmatt Dec 4, 2018

KtorZ Dec 5, 2018

KtorZ Dec 4, 2018

parsonsmatt commented Dec 4, 2018

iohk-bors bot commented Dec 4, 2018

parsonsmatt commented Dec 4, 2018

iohk-bors bot commented Dec 5, 2018

parsonsmatt commented Dec 5, 2018

iohk-bors bot commented Dec 5, 2018

Node API Documentation Server #3925

Node API Documentation Server #3925

Conversation

parsonsmatt commented Dec 4, 2018 • edited Loading

Description

Linked issue

Type of change

Developer checklist

Testing checklist

QA Steps

Screenshots (if available)

How to merge

KtorZ left a comment

Choose a reason for hiding this comment

KtorZ Dec 4, 2018

Choose a reason for hiding this comment

KtorZ Dec 4, 2018

Choose a reason for hiding this comment

KtorZ Dec 4, 2018

Choose a reason for hiding this comment

KtorZ Dec 4, 2018

Choose a reason for hiding this comment

parsonsmatt Dec 4, 2018

Choose a reason for hiding this comment

KtorZ Dec 5, 2018

Choose a reason for hiding this comment

KtorZ Dec 4, 2018

Choose a reason for hiding this comment

parsonsmatt commented Dec 4, 2018

iohk-bors bot commented Dec 4, 2018

Build failed

parsonsmatt commented Dec 4, 2018

iohk-bors bot commented Dec 5, 2018

Build failed

parsonsmatt commented Dec 5, 2018

iohk-bors bot commented Dec 5, 2018

Build succeeded

parsonsmatt commented Dec 4, 2018 •

edited

Loading