Skip to content

Conversation

@emostov
Copy link
Contributor

@emostov emostov commented Aug 27, 2020

This PR adds an OpenApi UI that will be hosted by github pages. The plans for v1.0.0 release are reflected in openapi-v1.yaml, while the plans for routes to add in v1.1.0 are reflected in openapi-v1-todo.yaml. The UI is currently set to show the v1.0.0 paths, which are not yet fully implemented.

Relates to #227

@emostov emostov added B0 - Silent PR should not be mentioned in release notes I5 - Documentation 📖 Documentation needs fixes or additions labels Aug 27, 2020
@github-pages github-pages bot temporarily deployed to github-pages August 27, 2020 18:20 Inactive
@github-pages github-pages bot temporarily deployed to github-pages August 27, 2020 18:22 Inactive
add docs:build to package.json

Small updates

Update gitignore

Update

update eslintignore

update author
@github-pages github-pages bot temporarily deployed to github-pages August 27, 2020 18:22 Inactive
@github-pages github-pages bot temporarily deployed to github-pages August 27, 2020 18:24 Inactive
@emostov emostov mentioned this pull request Aug 31, 2020
19 tasks
Copy link
Contributor

@danforbes danforbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you testing this? I don't know much about hosting stuff like this through GitHub Pages. My first inclination was to pull the branch down locally and try to access the UI that way but I realized that's not how things are set up.

@emostov
Copy link
Contributor Author

emostov commented Aug 31, 2020

@danforbes You should be able to pull the branch down and test it that way. Its in docs/dist/index.html. You will need to run yarn build:docs if you make any changes. For now I am more looking for feedback on the setup of the UI components and not the exact api specs since some still might change given the open PRs.

Also you can access the version from this branch here: https://paritytech.github.io/substrate-api-sidecar/dist/

Copy link
Contributor

@danforbes danforbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the directory structure and everything looks fine. I would suggest maybe adding a better README to the docs folder to explain where the actual OpenAPI spec is and what the other folders are for. Anything else in particular you were curious about?

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all calls have error descriptions – does that mean they can't fail? In general this lgtm a part from a few nits.

@emostov
Copy link
Contributor Author

emostov commented Sep 2, 2020

Not all calls have error descriptions – does that mean they can't fail? In general this lgtm a part from a few nits.

In theory we only have to detail all known errors. So essentially all 4xx errors should be detailed and some 5xx that are hard coded. I actually have not gone through the code and the specs though to make sure they align. I mostly just have errors from middleware that apply broadly. If we want to be thorough this definitely needs to be looked at carefully.

@github-pages github-pages bot temporarily deployed to github-pages September 5, 2020 22:39 Inactive
@github-pages github-pages bot temporarily deployed to github-pages September 5, 2020 22:42 Inactive
@github-pages github-pages bot temporarily deployed to github-pages September 5, 2020 22:43 Inactive
authId: oauth2.auth.name,
source: "auth",
level: "warning",
message: "Authorization may be unsafe, passed state was changed in server Passed state wasn't returned from auth server"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confusing

@dvdplm
Copy link
Contributor

dvdplm commented Sep 7, 2020

In theory we only have to detail all known errors. So essentially all 4xx errors should be detailed and some 5xx that are hard coded. I actually have not gone through the code and the specs though to make sure they align. I mostly just have errors from middleware that apply broadly. If we want to be thorough this definitely needs to be looked at carefully.

It's ok to defer this to a separate PR. Can you file a ticket?

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@github-pages github-pages bot temporarily deployed to github-pages September 7, 2020 14:14 Inactive
@github-pages github-pages bot temporarily deployed to github-pages September 7, 2020 18:09 Inactive
@emostov emostov merged commit 53a9dc1 into master Sep 7, 2020
@emostov emostov deleted the zeke-ui branch September 7, 2020 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B0 - Silent PR should not be mentioned in release notes I5 - Documentation 📖 Documentation needs fixes or additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants