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

mark_process_dead: avoid error with bad env #768

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

romuald
Copy link
Contributor

@romuald romuald commented Feb 9, 2022

When the multiprocess environemnt is not properly set-up (missing
environment variable) and mark_process_dead is called, the method raises
an error on path.join() because the path is None which may be
problematic

This commit simply ignores the cleanup when the variable is not present

Context

We are using the same library for multiprocess and non-multiprocess setups.

While the setup is obviously wrong on our part (adding mark_process_dead using atexit). I thought that check in prometheus_client would be nice addition if anyone would get in the same pitfall

When the multiprocess environemnt is not properly set-up (missing
environment variable) and mark_process_dead is called, the method raises
an error on path.join() because the path is None which may be
problematic

This commit simply ignores the cleanup when the variable is not present

Signed-off-by: Romuald Brunet <romuald@gandi.net>
@romuald romuald force-pushed the mark_process_dead-without-env branch from 1fbdd40 to d8d4248 Compare February 9, 2022 10:46
Copy link
Member

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

Thanks!

My initial reaction is that the error is actually a good thing though as it tells a user that they should fix their environment. At a minimum it seems like some sort of warning should be output?

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