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.

Sign up for GitHub

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

Jump to bottom

*: Never restart any service #304

Merged
merged 1 commit into from
Mar 12, 2024
Merged

*: Never restart any service #304

merged 1 commit into from
Mar 12, 2024

Conversation

carpawell
Copy link
Member

If a user runs services in the devenv it is most likely that they want it to work and serve for some purposes trying their best. Unconditional restart is also smth that you usually can see on a real machines.

@carpawell carpawell self-assigned this Dec 22, 2023
roman-khimov
roman-khimov reviewed Dec 26, 2023
View reviewed changes
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

But it's a dev-env. It's used for development. If something fails, I'd like to see it immediately (I'm developing the thing), not masking the behavior with restart (which if OK for production).

@carpawell
Copy link
Member Author

If something fails

Then it should be zero exit code. If it exits with non-zero code, nothing changes with this PR.

not masking the behavior with restart (which if OK for production).

Do not see any differences. In both you should go with caution, you have logs, you have docker stats (uptime), healthchecks.

I do not insist, but on-failure is senseless to me (moreover, not every service has it).

@roman-khimov
Copy link
Member

Maybe it should be "never" then?

@carpawell
Copy link
Member Author

Maybe it should be "never" then?

That is better to me than on-failure (if every service has it). At least that means something.

@carpawell
Copy link
Member Author

@roman-khimov, let's do something here. never and merge or just closing?

@roman-khimov
Copy link
Member

I'd ask @cthulhu-rider first of all. IMO never is the best option, but I'm not using dev-env that often.

@smallhive
Copy link
Contributor

I suffer from such restarts as well. After laptop restart dev-env in broken state and you have to drop and start again

@carpawell carpawell force-pushed the fix/always-restart-services branch from 441cbf7 to a74a5e1 Compare March 4, 2024 15:41
@carpawell
Copy link
Member Author

carpawell commented Mar 4, 2024 •
edited

Changed to never, @ALL, make your choices.

@carpawell
Copy link
Member Author

Well, not exactly that all...

@roman-khimov
Copy link
Member

@nspcc-dev, @nspcc-dev/neofs-core, @nspcc-dev/neofs-services

roman-khimov
roman-khimov previously approved these changes Mar 4, 2024
View reviewed changes
smallhive
smallhive previously approved these changes Mar 5, 2024
View reviewed changes
cthulhu-rider
cthulhu-rider previously approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

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

since we changed the solution, lets rename the PR also

@cthulhu-rider
Copy link
Contributor

btw is never a correct setting? https://docs.docker.com/compose/compose-file/compose-file-v2/#restart

seems like it should be no which is default

@carpawell carpawell changed the title *: Always restart every service *: Never restart any service Mar 6, 2024
@carpawell
*: Never restart any service
f585bcf 
Make it scream about its breaking state. A stopped service it easier to find,
its logs are clearer and running some tests will not hide any error that may
happen only once in a while.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
@carpawell carpawell force-pushed the fix/always-restart-services branch from a74a5e1 to f585bcf Compare March 6, 2024 11:01
@roman-khimov roman-khimov merged commit 0d09d7a into nspcc-dev:master Mar 12, 2024
1 check passed
@carpawell carpawell deleted the fix/always-restart-services branch March 12, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-khimov roman-khimov roman-khimov approved these changes

@smallhive smallhive Awaiting requested review from smallhive

@cthulhu-rider cthulhu-rider Awaiting requested review from cthulhu-rider cthulhu-rider is a code owner

Assignees

@carpawell carpawell

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@carpawell @roman-khimov @smallhive @cthulhu-rider