-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add swagger-ui into docker network to allow viewing Canton JSON API V2 Open API spec in browser #44
base: main
Are you sure you want to change the base?
Conversation
server_name swagger.localhost; | ||
location /docs/openapi { | ||
proxy_pass http://participant-app-provider:7575/docs/openapi; | ||
add_header Access-Control-Allow-Origin *; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added headers here to allow CORS, avoiding the issue mentioned here
swagger-ui: | ||
image: swaggerapi/swagger-ui | ||
ports: | ||
- "9080:8080" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
port to 9080 as 8080 is used for backend service
already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if it's necessary to make this port configurable in .env file 🤔 , lemme know if you think that's better. 🙏
start-swagger-ui: | ||
docker compose -f docker/swagger/compose.yaml $(DOCKER_COMPOSE_ENVFILE) up -d || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to remove the -d
flag and start the service in the foreground rather than having start/stop/restart/clean
lifecycle targets (I would remove the other ones). It can then be terminated conventionally with ctrl-c
. I think this is enough because the UI doesn't interact directly with the application as a service.
In that case it would make sense to log/@echo
out the URL to open the UI before startup from the make
target.
If instead you prefer the existing lifecycle targets, please also add a clean
-target and depend on it from clean-docker
.
In either case please also add the ##
docstrings so that the targets show up in make help
🙏
Purpose as titled
The PR is aimed to address the issue proposed here