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

Server should exit with non-0 on error #16476

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

martimors
Copy link

@martimors martimors commented Dec 22, 2024

Closes #16474 .

Just sys.exit(process.returncode) would be fine here, but for the sake of consistency with the rest of the code, I went for the exit_with_error utility. The server process is expected to run for ever until stopped, so we should never get a 0 exit code from the process, hence there is no need for a ternary condition here.

I also went ahead and removed the bare except right above - all it did was log the error and continue, and this only hides potential errors and causes issues similar to the one I faced.

In my view, this is a bugfix, but I suppose some clients could depend on the 0 exit code here in theory? I have not familiarized with your versioning process so let me know if I should bump the patch version or something as part of this.

Help needed: Looks like my test fails because the settings object is not re-created between tests, thus my monkeypatch call has no effect and my test fails (passes locally but fails in CI). Could you advise if there is a way to mess up the config temporarily without impacting downstream tests?

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

@github-actions github-actions bot added the bug Something isn't working label Dec 22, 2024
Copy link

codspeed-hq bot commented Dec 22, 2024

CodSpeed Performance Report

Merging #16476 will not alter performance

Comparing martimors:bugfix/exit-with-non-0-on-error (7e83899) with main (46c6164)

Summary

✅ 3 untouched benchmarks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server command exits with exit code 0 even when failing due to database connectivity
1 participant