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

3148 add swaggo for swagger directory for rest #3177

Closed
wants to merge 19 commits into from

Conversation

aronchick
Copy link
Collaborator

No description provided.

@aronchick aronchick linked an issue Jan 6, 2024 that may be closed by this pull request
Comment on lines +41 to +69
//// @securityDefinitions.basic BasicAuth

//// @securityDefinitions.apikey ApiKeyAuth
//// @in header
//// @name Authorization
//// @description Description for what is this security definition being used

// -- Add authentication to swagger here
//// @securitydefinitions.oauth2.application OAuth2Application
//// @tokenUrl https://example.com/oauth/token
//// @scope.write Grants write access
//// @scope.admin Grants read and write access to administrative information
//
//// @securitydefinitions.oauth2.implicit OAuth2Implicit
//// @authorizationUrl https://example.com/oauth/authorize
//// @scope.write Grants write access
//// @scope.admin Grants read and write access to administrative information
//
//// @securitydefinitions.oauth2.password OAuth2Password
//// @tokenUrl https://example.com/oauth/token
//// @scope.read Grants read access
//// @scope.write Grants write access
//// @scope.admin Grants read and write access to administrative information
//
//// @securitydefinitions.oauth2.accessCode OAuth2AccessCode
//// @tokenUrl https://example.com/oauth/token
//// @authorizationUrl https://example.com/oauth/authorize
//// @scope.admin Grants read and write access to administrative information

Copy link
Contributor

@simonwo simonwo Jan 10, 2024

Choose a reason for hiding this comment

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

Not correct for us – we don't implement authorization using OAuth2 at the moment, and these are example URLs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is all commented out, fyi

Comment on lines +70 to +79
// NodeInfo godoc
//
// @ID NodeInfo
// @Summary Information about the node.
// @Description Information about the node.
// @Tags Ops
// @Produce json
// @Success 200 {object} NodeInfo
// @Failure 500 {object} string
// @Router /api/v1/agent/node [get]
Copy link
Contributor

Choose a reason for hiding this comment

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

Swagger comments should be added on the actual method which serves the request, rather than the data structure, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Swagger needs the godoc but you're right (I think) no router here

// @Success 200 {object} []peer.AddrInfo
// @Success 200 {object} []string
Copy link
Contributor

Choose a reason for hiding this comment

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

peer.AddrInfo is not a string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a mechanism to flatten a complex structure into a string - swaggo does this automatically. If we'd like to do this more thoroughly, we can, but I get it was overkill

@aronchick aronchick marked this pull request as draft January 11, 2024 06:23
@aronchick
Copy link
Collaborator Author

Replaced with this stack of PRs - #3224

@aronchick aronchick closed this Jan 11, 2024
@aronchick aronchick deleted the 3148-add-swaggo-for-swagger-directory-for-rest branch January 11, 2024 19:29
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.

Add swaggo for swagger directory for REST
2 participants