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

Configurable Number of Lines in Healthcheck Log Output #444

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

rience
Copy link
Contributor

@rience rience commented Sep 5, 2023

While I was configuring Kamal for our deployments I was limited by number of lines that (by default) Kamal is printing out when running healthcheck. It is hardcoded to "50" and quite often I was only looking at the bottom of stack trace and could not figure out what has gone wrong.

Container is removed after failed deployment and I couldn't find a way to get these logs from anywhere.

So this PR is adding configurable number of lines for healthcheck.

@rience rience force-pushed the custom-healthcheck-log-lines-count branch from ecd58ed to 053b9d3 Compare September 6, 2023 08:54
@djmb
Copy link
Collaborator

djmb commented Sep 12, 2023

Hi @rience, thank you for the PR!

There are some conflicts, if you could resolve them when you have time?

Also what do you think about changing the setting name to log_lines?

@rience rience force-pushed the custom-healthcheck-log-lines-count branch from 053b9d3 to 892cf0e Compare September 12, 2023 19:06
@rience
Copy link
Contributor Author

rience commented Sep 12, 2023

Hi @djmb,

That's rebased and name of setting has changed to log_lines. There is one test that is failing - I cannot see how this is connected to my test though.

@djmb
Copy link
Collaborator

djmb commented Sep 13, 2023

@rience - The tests passed on a re-run. The integration tests are running with docker inside docker inside a GitHub action so we do get the odd random failure. Thanks for the PR!

@djmb djmb merged commit 60835d1 into basecamp:main Sep 13, 2023
6 checks passed
@rience
Copy link
Contributor Author

rience commented Sep 13, 2023

Sure - one more thing @djmb - I've also added this short description to kamal-site: basecamp/kamal-site#28

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.

2 participants