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

refactor: consolidate all settings in defaultProcessSettings #348

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

shivaraj-bh
Copy link
Member

resolves #317

@shivaraj-bh shivaraj-bh changed the title refactor: consolidate all settings in `defaultProcessSettings" refactor: consolidate all settings in defaultProcessSettings Oct 4, 2024
@shivaraj-bh shivaraj-bh merged commit 1e682a8 into main Oct 4, 2024
2 checks passed
@shivaraj-bh shivaraj-bh deleted the common-settings branch October 4, 2024 16:31
@shivaraj-bh
Copy link
Member Author

shivaraj-bh commented Oct 27, 2024

This PR will have to be reverted because it falsely assumes that even processes other than the one named name (name from services.<service>.<name>) also takes the same defaults, which is not true.

For example, when <name>-init process of postgres fails, the default setting, availability.restart, will restart the process, even though that is not the desired behaviour.

This wasn’t caught in CI because we don’t test failure of init process of any service, guess I will add them in the PR that reverts this change.

shivaraj-bh added a commit that referenced this pull request Oct 27, 2024
shivaraj-bh added a commit that referenced this pull request Oct 27, 2024
shivaraj-bh added a commit that referenced this pull request Nov 4, 2024
shivaraj-bh added a commit that referenced this pull request Nov 4, 2024
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.

Re-evaluate max_restarts
2 participants