Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Update docker-compose.yml #156

Closed
wants to merge 1 commit into from
Closed

Update docker-compose.yml #156

wants to merge 1 commit into from

Conversation

Angelo-gh3990
Copy link
Contributor

Change this, and now it builds correctly. Don't know if this is what you want?

ref: #155

Change this, and now it builds correctly. Don't know if this is what you want?

ref: #155
@@ -132,7 +132,7 @@ services:
- motion-singularity-volume:/usr/src/app/storage
depends_on:
singularity_api:
condition: service_healthy
condition: service_completed_successfully
Copy link
Collaborator

Choose a reason for hiding this comment

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

Motion depends on singularity being up and running, not just that it has run. So, it seems like it would be more correct to add a health check to singularity, and keep this as condition: service_healthy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I agree, after looking into it more. It showed me for now where the issue was. Thanks

Copy link
Collaborator

@xinaxu xinaxu Oct 12, 2023

Choose a reason for hiding this comment

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

There is already a /health endpoint to check for health in Singularity
https://github.com/data-preservation-programs/singularity/blob/7a3b2c61c4deb38aea50b6b8bd17a5ab75ff420d/api/api.go#L437C37-L437C37

So something like below

test: curl --fail http://localhost:9090/health || exit 1
interval: 2s
retries: 5
start_period: 2s
timeout: 10s

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that's only available with v0.5.0, below one should work though
http://127.0.0.1:9090/swagger/index.html

Copy link
Collaborator

@xinaxu xinaxu left a comment

Choose a reason for hiding this comment

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

see above

@gammazero
Copy link
Collaborator

gammazero commented Oct 18, 2023

@xinaxu So I think we can close this as it should be fixed by #164.

@xinaxu xinaxu closed this Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants