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

Playground with go swagger #265

Merged
merged 11 commits into from Jun 21, 2018

Conversation

Projects
None yet
3 participants
@interro
Copy link
Contributor

commented Jun 11, 2018

install go swagger:

brew tap go-swagger/go-swagger
brew install go-swagger

go swagger docs:
https://goswagger.io/

generate tequila-api spec and serve it locally:
bin/swagger_serve_doc

@interro interro requested review from donce, tadovas and Waldz as code owners Jun 11, 2018

//
// in: path
// required: true
Id string `json:"id"`

This comment has been minimized.

Copy link
@donce

donce Jun 11, 2018

Contributor

ID

@@ -29,10 +29,23 @@ import (
"github.com/mysterium/node/tequilapi/validation"
)

// Used for operations that want identity in the path
// swagger:parameters registerIdentity
type IdentityURLParams struct {

This comment has been minimized.

Copy link
@donce

donce Jun 11, 2018

Contributor

Is it possible to re-use this struct inside code when parsing request params?

This comment has been minimized.

Copy link
@interro

interro Jun 12, 2018

Author Contributor

I think it should be possible

This comment has been minimized.

Copy link
@interro

interro Jun 12, 2018

Author Contributor

found way to declare parameters in other way, so we no longer need struct IdentityURLParams

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

I agree, would be good to investigate parameters more. https://goswagger.io/generate/spec/params.html

"type": "object",
"x-go-package": "github.com/mysterium/node/tequilapi/validation"
},
"IdentityURLParams": {

This comment has been minimized.

Copy link
@donce

donce Jun 11, 2018

Contributor

Is this definition used anywhere?

This comment has been minimized.

Copy link
@interro

interro Jun 12, 2018

Author Contributor

IdentityURLParams is never used in go code, it is only for swagger to declare a parameter

#!/bin/bash

swagger generate spec --base-path=./cmd/mysterium_client/ -o ./tequilapi_spec.json --scan-models
swagger serve ./tequilapi_spec.json -F=swagger

This comment has been minimized.

Copy link
@donce

donce Jun 11, 2018

Contributor

Instead of brew, we can download swagger from sources, adding it as glide dependency:

go get -u github.com/go-swagger/go-swagger/cmd/swagger
@@ -0,0 +1,4 @@
#!/bin/bash

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

No need for separate HTTP server, lets create ticket to serve Swagger documentation right in Tequilapi path /

server.ginEngine.StaticFile("/", "public/index.html")
server.ginEngine.Static("/js", "public/js")
server.ginEngine.Static("/swagger", "public/swagger.json")
@@ -0,0 +1,206 @@
{
"consumes": [
"application/json"

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

I guess we shouldn't commit static file if it's generated from code

//
// Version: 0.0.1
//
// Consumes:

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

Are these simple text like "Consumes" real annotations?
You can also have file swagger-header.yml during generation phase which would play a role of default values

@@ -100,6 +124,17 @@ func (endpoint *identitiesAPI) Create(resp http.ResponseWriter, request *http.Re
utils.WriteAsJSON(idDto, resp)
}

// swagger:route PUT /identities/:id/registration Identity registerIdentity

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

Would be great path documentation to be next to route mapping

ID string `json:"id"`
}

// Some info

This comment has been minimized.

Copy link
@Waldz
// Used for operations that want identity in the path
// swagger:parameters registerIdentity
type IdentityURLParams struct {
// Identity to pass

This comment has been minimized.

Copy link
@Waldz
type IdentityURLParams struct {
// Identity to pass
//
// in: path

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

Looks like this should be some swagger annoations

//
// Create new identity
//
// Inner description

This comment has been minimized.

Copy link
@Waldz

This comment has been minimized.

Copy link
@interro

interro Jun 13, 2018

Author Contributor

these are valid for swagger, but changed to named values for more readability

}

// Some info
// swagger:model
type identityDto struct {
ID string `json:"id"`

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

I miss fields description and examples of data

This comment has been minimized.

Copy link
@interro

interro Jun 13, 2018

Author Contributor

added

@@ -29,10 +29,23 @@ import (
"github.com/mysterium/node/tequilapi/validation"
)

// Used for operations that want identity in the path
// swagger:parameters registerIdentity
type IdentityURLParams struct {

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 12, 2018

Member

I agree, would be good to investigate parameters more. https://goswagger.io/generate/spec/params.html

@interro interro changed the title Playground with go swagger. Not finished. Playground with go swagger Jun 13, 2018

//
// API description.
//
// Version: 0.0.1

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 13, 2018

Member

We dont have API versioning yet

// swagger:operation PUT /connection Connection createConnection
// ---
// summary: Starts new connection
// description: Inner description

This comment has been minimized.

Copy link
@Waldz
// description: Connection started
// schema:
// "$ref": "#/definitions/statusResponse"
// 400:

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 14, 2018

Member

422 missing

"description": "API description.",
"title": "Tequila API.",
"version": "0.0.1"
"description": "The purpose of this documentation is to provide developers an insight of how to\ninteract with Mysterium Node via Tequila API.\nThis should demonstrate all the possible API calls with described parameters and responses.",

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 14, 2018

Member

I would not commit this file, and run generation in CI as soon as possible

This comment has been minimized.

Copy link
@interro

interro Jun 14, 2018

Author Contributor

will not commit this file

"notImplemented": {
"description": "Not Implemented"
"errorMessage": {
"headers": {

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 14, 2018

Member

It's not in headers, but in body

This comment has been minimized.

Copy link
@interro

interro Jun 14, 2018

Author Contributor

fixed

@donce
Copy link
Contributor

left a comment

Please add instructions how to setup and run it.

@@ -42,7 +42,7 @@ MESSAGES_RECONFIGURE=()

check_uncleaned_package "github.com/mysterium/node/cmd" 3
check_uncleaned_package "github.com/mysterium/node/identity" 9
check_uncleaned_package "github.com/mysterium/node/tequilapi" 10
check_uncleaned_package "github.com/mysterium/node/tequilapi" 12

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 15, 2018

Member

Why? My incentivisation does not work :/

interro added some commits Jun 11, 2018

@interro interro dismissed stale reviews from Waldz and donce via 240123c Jun 20, 2018

@interro interro force-pushed the feature/MYST-557-api-doc branch from 1eee2df to 240123c Jun 20, 2018

@donce

donce approved these changes Jun 21, 2018

@Waldz

Waldz approved these changes Jun 21, 2018

@interro interro merged commit 52905b5 into master Jun 21, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@interro interro deleted the feature/MYST-557-api-doc branch Jun 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.