Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor: start dashmate services depending on status #503
refactor: start dashmate services depending on status #503
Changes from 8 commits
3c6c3a3
ba6a4cf
d08a3b7
42227a9
1eb6539
12a8d98
51c5968
788e861
1adec64
b656904
7fff8ce
e9cb556
414a3e9
a4bbad3
02b08e5
0c92a06
1b8484c
b5dbe2a
7848224
0ce0f7d
4a06b95
f02b30d
cec1346
b51fb42
1bd0879
371c2ba
21c6582
bbd8e38
5ad5dfd
cdd4049
fa503a0
ec8f134
883475f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It seems we need to get all platform service statuses otherwise it will be inconsistency and conflict with the check in dashmate https://github.com/dashpay/platform/blob/2e984df0302fa94bdf187bd893328ec58f390aa0/packages/dashmate/src/docker/DockerCompose.js#L75
We using this function in start and restart commands I believe
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.
Don't we need to restart Core as well? What if dashmate code is updated or config is changed?
We can do restart logic smarter:
@pshenmic @strophy What do you think guys?
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.
Right, yesterday I had difficulty updating from Platform 0.24.2 to 0.24.4 because
docker compose restart
does not pull and use newer images,docker compose up -d
is necessary for this. We definitely need a way of ensuring the necessary containers are restarted (or stopped, updated and started) when config changes. But changing the dashmate version probably shouldn't cause core to restart - this usually changes at the same time as the platform version only.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.
it's not, that's why we have
dashmate update
aboveWe should because docker compose config could be changed.
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.
What do you think about this logic: