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.

Sign up for GitHub

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

Jump to bottom

[#114] Swagger API generator #115

Merged
merged 5 commits into from
Aug 22, 2022
Merged

[#114] Swagger API generator #115

merged 5 commits into from
Aug 22, 2022

Conversation

DK318
Copy link
Member

@DK318 DK318 commented May 23, 2022 •
edited
Loading

Description

Problem

We don't have documentation for our coffer Web API.

Solution

Added executable coffer-swagger-api which generates
Swagger docs.

Related issue(s)

Fixed #

✅ Checklist for your Pull Request

Related changes (conditional)

  • Tests

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.

  • Documentation

    • I checked whether I should update the docs and did so if necessary:

Stylistic guide (mandatory)

sancho20021
sancho20021 suggested changes May 23, 2022
View reviewed changes
Copy link
Contributor

@sancho20021 sancho20021 left a comment

Choose a reason for hiding this comment

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

I tried importing build swagger.json using swagger website, and some errors were shown. Is that normal?
image

lib/Entry.hs Outdated Show resolved Hide resolved
lib/Entry.hs Show resolved Hide resolved
@DK318
Copy link
Member Author

DK318 commented May 23, 2022 •
edited
Loading

I tried importing build swagger.json using swagger website, and some errors were shown. Is that normal?

They are produced because of bad ToSchema instances. They would be removed when #112 is merged.

@DK318 DK318 force-pushed the dk318/#114-swagger-api branch from cf4ac95 to 4bd120e Compare May 28, 2022 13:55
dcastro
dcastro requested changes Jun 6, 2022
View reviewed changes
app/swagger-api/Main.hs Outdated Show resolved Hide resolved
app/swagger-api/Main.hs Outdated Show resolved Hide resolved
lib/Entry.hs Outdated Show resolved Hide resolved
lib/BackendName.hs Outdated Show resolved Hide resolved
lib/CLI/Types.hs Outdated Show resolved Hide resolved
app/swagger-api/Main.hs Show resolved Hide resolved
lib/Backend/Vault/Kv/Internal.hs Outdated Show resolved Hide resolved
lib/Coffer/Path.hs Outdated Show resolved Hide resolved
lib/Coffer/Path.hs Show resolved Hide resolved
@sancho20021
Copy link
Contributor

Ok now tests don't pass. Maybe the problem was fixed after #113 had been closed, so rebase on main will help.

Firstly I will address the rest of the comments, and then rebase and try to fix the CI tests fails

sancho20021 added a commit that referenced this pull request Jul 25, 2022
@sancho20021
[#114] Fix swagger structural error
fa353ce 
Problem: list of tuples - [(EntryPath, EntryPath)] - used in
the WebAPI for `copy` and `rename` causes errors in Swagger docs.
See #115 (comment)

Solution: introduce type `PairObject` just to fix this error.
@sancho20021
Copy link
Contributor

I've tried to rebase, but I didn't manage to. This error appears after rebasing the first commit:

app/swagger-api/Main.hs:17:17: error:
    • No instance for (ToParamSchema Coffer.Path.EntryPath)
        arising from a use of ‘toOpenApi’
    • In the first argument of ‘(&)’, namely
        ‘toOpenApi (Proxy :: Proxy API)’
      In the first argument of ‘(&)’, namely
        ‘toOpenApi (Proxy :: Proxy API) & info . title .~ "Coffer Web API"’
      In the first argument of ‘(&)’, namely
        ‘toOpenApi (Proxy :: Proxy API) & info . title .~ "Coffer Web API"
           & info . version .~ "1.0"’
   |
17 |   let openApi = toOpenApi (Proxy :: Proxy API)

I think after the commits are approved, I can squash everything except two last commits, perhaps it will make the rebase easier.

@sancho20021 sancho20021 requested a review from dcastro July 26, 2022 08:46
dcastro pushed a commit that referenced this pull request Aug 20, 2022
@sancho20021 @dcastro
[#114] Fix swagger structural error
84e24d5 
Problem: list of tuples - [(EntryPath, EntryPath)] - used in
the WebAPI for `copy` and `rename` causes errors in Swagger docs.
See #115 (comment)

Solution: introduce type `PairObject` just to fix this error.
@dcastro dcastro force-pushed the dk318/#114-swagger-api branch from 6ae301a to 4f92bfb Compare August 20, 2022 15:44
@dcastro dcastro force-pushed the dk318/#114-swagger-api branch 2 times, most recently from 8220515 to 421a615 Compare August 21, 2022 19:15
DK318 and others added 5 commits August 22, 2022 08:52
@DK318 @dcastro
[#114] Swagger API generator
18a350e 
Problem: we don't have documentation for our `coffer Web API`.

Solution: added executable `coffer-swagger-api` which generates
Swagger docs.
@dcastro
[#114] Add CORS support
ac0bb18
@dcastro
[#114] Delete EntryConvertible
7fc24f3 
Problem: This typeclass is not used anywhere.

Solution: Delete it.
@dcastro
[#114] Add link to swagger docs in the readme
edfe5f8
@dcastro
[#114] Check swagger docs are up to date
ad1d1a5
@dcastro dcastro force-pushed the dk318/#114-swagger-api branch from b6bb197 to ad1d1a5 Compare August 22, 2022 07:54
@dcastro
Copy link
Member

dcastro commented Aug 22, 2022

Ok, I've added:

  • Examples for most schemas
  • Descriptions to most query parameters
  • CORS support (so we can use the "Try it out" and "Execute" buttons in the Swagger UI)
  • A link to the swagger docs in the readme
  • A CI step to make sure docs/swagger.json is up-to-date

@dcastro
Copy link
Member

dcastro commented Aug 22, 2022

@sancho20021 can you approve please? ^^

@dcastro
Copy link
Member

dcastro commented Aug 22, 2022

The docs for this branch can be seen here: https://petstore.swagger.io/?url=https://raw.githubusercontent.com/serokell/coffer/dk318/%2523114-swagger-api/docs/swagger.json#/

@sancho20021
Copy link
Contributor

@sancho20021 can you approve please? ^^

@dcastro Sure! Just approve or do a review and approve?

@dcastro
Copy link
Member

dcastro commented Aug 22, 2022

Just approve or do a review and approve?

@sancho20021 If you have the time, a review would be welcome as well 😄 if not, that's fine too.

sancho20021
sancho20021 approved these changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@sancho20021 sancho20021 left a comment

Choose a reason for hiding this comment

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

@dcastro Ok. I'm on vacation 😄 so I'll let myself just approve)

@dcastro dcastro merged commit 70ba963 into main Aug 22, 2022
@dcastro dcastro deleted the dk318/#114-swagger-api branch August 22, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sancho20021 sancho20021 sancho20021 approved these changes

@dcastro dcastro dcastro approved these changes

@Kariel-Myrr Kariel-Myrr Awaiting requested review from Kariel-Myrr

Assignees

@sancho20021 sancho20021

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generate documentation for the Web API
3 participants
@DK318 @sancho20021 @dcastro