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

Or 1101 titond backend add swagger and ci for checking if the swagger doc is updated #10

Conversation

nguyenzung
Copy link
Member

  • Setup swagger
  • Check if there is APIs without Swagger comment

Thank you!

@rlgns98kr
Copy link
Member

image
I think we can count handler except for swagger (just do -1). Then, action will be failed without swagger for each API (not same the number of swagger and handler).

Will action be failed without swagger for each API now?

@nguyenzung
Copy link
Member Author

image I think we can count handler except for swagger (just do -1). Then, action will be failed without swagger for each API (not same the number of swagger and handler).

Will action be failed without swagger for each API now?

That is exactly the way i am implementing. I compare the number of api in swagger docs and the total APIs - 1.
Now, action will failed if there is APIs without swagger docs

docs/docs.go Outdated Show resolved Hide resolved
Dockerfile Outdated
@@ -7,6 +7,7 @@ WORKDIR /app
COPY . .

ARG TARGETOS TARGETARCH
RUN go install github.com/swaggo/swag@latest
Copy link
Member

@rlgns98kr rlgns98kr Nov 28, 2023

Choose a reason for hiding this comment

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

I think we don't have to add this. we are not using swag command in docker container!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!
In actual, i also was not sure about that!

Copy link
Member

@rlgns98kr rlgns98kr Nov 28, 2023

Choose a reason for hiding this comment

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

Thank you very much Brave! I think the command need to use swag command!
Instead of this, we have to add copy of api directory to /root/api!
@ohbyeongmin
And I'm sorry my missing about the directory of deployments. I gave fault path of it to you before. The path is /root/deployments!

Copy link
Member

Choose a reason for hiding this comment

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

@nguyenzung
Oh, we are using the swag command in Dockerfile,,, I know it now. But we have to add files for swagger.

Copy link
Member

Choose a reason for hiding this comment

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

FROM --platform=$BUILDPLATFORM golang:1.20-alpine AS builder

RUN apk add --no-cache make gcc musl-dev linux-headers git

WORKDIR /app

COPY . .

RUN go install github.com/swaggo/swag/cmd/swag@latest

ARG TARGETOS TARGETARCH
RUN GOOS=$TARGETOS GOARCH=$TARGETARCH make titond

FROM alpine

RUN apk add --no-cache ca-certificates jq curl
COPY --from=builder /app/api /root/api
COPY --from=builder /app/deployments /root/deployments
COPY --from=builder /app/build/bin/titond /usr/local/bin/

WORKDIR /root
ENTRYPOINT ["titond"]

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you very much for your support
I updated and also update the base image due this issue: aws/aws-cli#4971

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!
But I think we don't have to get aws-cli in our container! If you agree, let me change it later!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, thank you,
I added aws-cli because I couldnot run in my laptop

cmd/titond/main.go Outdated Show resolved Hide resolved
@nguyenzung nguyenzung force-pushed the OR-1101-titond-backend-add-swagger-and-ci-for-checking-if-the-swagger-doc-is-updated branch from c403cc5 to 4707570 Compare November 28, 2023 08:21
@@ -10,6 +10,7 @@ import (
"github.com/tokamak-network/tokamak-titond-backend/pkg/http"
"github.com/tokamak-network/tokamak-titond-backend/pkg/kubernetes"
"github.com/tokamak-network/tokamak-titond-backend/pkg/services"
apptest "github.com/tokamak-network/tokamak-titond-backend/test"
Copy link
Member

Choose a reason for hiding this comment

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

I think we can use test as a package name now. It is small thing. Thank you! 😍

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated

@@ -107,6 +107,7 @@ func checkSwagger(ctx *cli.Context) error {
fmt.Printf("%-6s %-25s %s\n", route.Method, route.Path, route.Handler)
}
fmt.Println("Total APIs: ", numAPIs)
fmt.Println("Total APIs excluding swagger api: ", (numAPIs - 1))
Copy link
Member

Choose a reason for hiding this comment

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

So kind! 😍

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you ^^

Copy link
Member

@rlgns98kr rlgns98kr left a comment

Choose a reason for hiding this comment

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

Thank you very much, Brave! ❤️❤️

COPY --from=builder /app/deployments /root/deployments
COPY --from=builder /app/build/bin/titond /usr/local/bin/

WORKDIR /usr/local/bin/
WORKDIR /root
Copy link
Member

Choose a reason for hiding this comment

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

@ohbyeongmin We can use the ./deployments path in container env now!

@nguyenzung nguyenzung merged commit 7ac5f40 into main Nov 29, 2023
2 checks passed
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.

None yet

2 participants