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

feat(openapi): docs for tsuro bind and bind-app #87

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

rafaelrubbioli
Copy link
Contributor

@rafaelrubbioli rafaelrubbioli commented Oct 16, 2020

I added openapi docs for some routes that were missing

POST: /resources/:instance/bind-app
DELETE: /resources/:instance/bind-app
POST: /resources/:instance/bind
DELETE: /resources/:instance/bind

I'm not familiar with openapi, so if there's anything i did wrong or should be improved, please let me know!

Part of #39

Copy link
Member

@nettoclaudio nettoclaudio left a comment

Choose a reason for hiding this comment

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

Hey @rafaelrubbioli. Welcome! Thanks for contributing w/ Tsuru.

I did some suggestions, can you address them?

@@ -179,6 +179,79 @@ paths:
'204':
description: Instance is up and running

/resources/{instance}/bind-app:
parameters:
- name: name
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: name
- name: instance


/resources/{instance}/bind:
parameters:
- name: name
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: name
- name: instance

Comment on lines 451 to 467
BindApp:
type: object
properties:
app-name:
type: string
app-host:
type: string
user:
type: string
eventid:
type: string
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the missing fields app-hosts and app-internal-hosts? The later was included on tsuru/tsuru#2442... there you can find some description about.

Providing some examples can help to improve this spec.

Comment on lines 237 to 258
post:
summary: Bind Unit
description: The service endpoint binds a unit to the service instance
tags:
- rpaas
responses:
'201':
description: Unit successfully bound to the service instance

delete:
summary: Unbind Unit
description: The service endpoint unbinds the unit from the service instance
tags:
- rpaas
responses:
'200':
description: Unit successfully unbound from service instance

Copy link
Member

Choose a reason for hiding this comment

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

We do nothing on these routes. We ever return 200 OK just to follow the Tsuru Service API spec... I would mark those as deprecated to indicate this.

'201':
description: Unit successfully bound to the service instance

delete:
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide the operation ID for this route?

schema:
type: string

post:
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide the operation ID for this route? +1

@rafaelrubbioli
Copy link
Contributor Author

@nettoclaudio Thank you for the tips, could you check if i got it right?

@nettoclaudio
Copy link
Member

Looks good, @rafaelrubbioli. We appreciate your contribution. Thanks!

@nettoclaudio nettoclaudio merged commit 4222851 into tsuru:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants