Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register swagger API #7246

Merged
merged 20 commits into from
Sep 19, 2020
Merged

Register swagger API #7246

merged 20 commits into from
Sep 19, 2020

Conversation

anilcse
Copy link
Collaborator

@anilcse anilcse commented Sep 7, 2020

Description

  • Move swagger docs from client/grpc-gateway to client/docs
  • Fix swagger generation to combine old api docs. Remove x/ibc docs from old swagger
  • Register swagger docs API in app.go, this allows other applications to override/customize swagger API easily

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Sep 7, 2020

Codecov Report

Merging #7246 into master will decrease coverage by 0.01%.
The diff coverage is 11.11%.

@@            Coverage Diff             @@
##           master    #7246      +/-   ##
==========================================
- Coverage   55.84%   55.82%   -0.02%     
==========================================
  Files         586      586              
  Lines       35967    35975       +8     
==========================================
  Hits        20084    20084              
- Misses      13914    13922       +8     
  Partials     1969     1969              

@anilcse anilcse added API T:Docs Changes and features related to documentation. labels Sep 11, 2020
@anilcse anilcse marked this pull request as ready for review September 11, 2020 19:48
simapp/app.go Show resolved Hide resolved
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK

@anilcse anilcse added the A:automerge Automatically merge PR once all prerequisites pass. label Sep 18, 2020
Copy link
Contributor

@clevinson clevinson left a comment

Choose a reason for hiding this comment

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

ACK! One small point about the names of temp folders created.

Also- is it possible to mark certain API endpoints (e.g. all legacy REST endpoints) as deprecated/soon-to-be-deprecated in Swagger docs?

Looking at the swagger UI it's a bit confusing which endpoints are part of legacy and which ones are the new protobuf ones. (yes I understand that anything with /cosmos or /ibc prefixes are proto, but this is not very intuitive for new developers.


# clean swagger files
find ./ -name 'query.swagger.json' -exec rm {} \;
rm -rf cosmos
rm -rf ibc
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we output these to a default root directory other than ./ ?

As it is now, this would cause problems if in the future we add a ./ibc or ./cosmos directory for any purpose besides these swagger auto generated files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also- is it possible to mark certain API endpoints (e.g. all legacy REST endpoints) as deprecated/soon-to-be-deprecated in Swagger docs?

Yes 100%, I'll address this in a followup PR

Can we output these to a default root directory other than ./ ?

As it is now, this would cause problems if in the future we add a ./ibc or ./cosmos directory for any purpose besides these swagger auto generated files.

Correct, but we don't have custom output directory support for protoc-sawgger-gen. I have created an issue upstream to add support for that. For now,

  • We can revert this to remove all generated files (find ./ -name 'query.swagger.json' -exec rm {} \;) first and then remove ./cosmos, ./ibc directories if they are empty. wdyt?

Copy link
Collaborator Author

@anilcse anilcse Sep 23, 2020

Choose a reason for hiding this comment

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

We have a way to override the output directory for swagger generated files. It;s being addressed here: #7374 & #7399

@mergify mergify bot merged commit ca7b31d into master Sep 19, 2020
@mergify mergify bot deleted the anil/serve_swagger branch September 19, 2020 00:34
@fedekunze
Copy link
Collaborator

looks good! how do I deploy the swagger docs locally?

@anilcse
Copy link
Collaborator Author

anilcse commented Sep 20, 2020

looks good! how do I deploy the swagger docs locally?

you can access it via http://localhost:1317/swagger/. By default swagger api is disabled, you can enable them by changing api.enable in app.toml

@fedekunze
Copy link
Collaborator

you can access it via http://localhost:1317/swagger/. By default swagger api is disabled, you can enable them by changing api.enable in app.toml

Is that documented somewhere? If not, can we add it as a todo item for the v0.40 docs?

@anilcse anilcse mentioned this pull request Sep 23, 2020
9 tasks
secret2830 pushed a commit to bianjieai/cosmos-sdk that referenced this pull request Sep 27, 2020
* init

* Fix statik gen

* Fix swagger

* Change swagger url

* Fix swagger serve

* remove ibc swagger from legacy docs

* Add old routes config

* Move swagger api to app.go

* add godoc

* Fix inputs

* Fix swagger dir

* Fix statik

* refactor

* fmt

* fix doc

* Fix swagger config check
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
* init

* Fix statik gen

* Fix swagger

* Change swagger url

* Fix swagger serve

* remove ibc swagger from legacy docs

* Add old routes config

* Move swagger api to app.go

* add godoc

* Fix inputs

* Fix swagger dir

* Fix statik

* refactor

* fmt

* fix doc

* Fix swagger config check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. T:Docs Changes and features related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants