Skip to content

Fix multi process collision by initializing pid variable in master process - #192

Merged
brian-brazil merged 3 commits into
prometheus:masterfrom
aimestereo:fix-multiprocessing-for-celery
Sep 3, 2017
Merged

brian-brazil merged 3 commits into
prometheus:masterfrom
aimestereo:fix-multiprocessing-for-celery

Conversation

@aimestereo

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread prometheus_client/core.py Outdated
files = {}
values = []

# 1. use dict instead of nonlocal keyword to support python 2.7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment isn't adding much to someone familiar with Python, I'd remove it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agree

Comment thread prometheus_client/core.py Outdated
def __check_for_pid_change(self):
if self._pid != _pidFunc():
actual_pid = _pidFunc()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can remove this blank line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment thread prometheus_client/core.py Outdated
files = {}
values = []

# pid needs to be evaluated here in a master process

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's usage dependant, not all relevant systems have masters. I think it'll be fine not to have this comment at all.

@aimestereo
aimestereo force-pushed the fix-multiprocessing-for-celery branch from b3d2f19 to b681e60 Compare September 2, 2017 20:26
@brian-brazil
brian-brazil merged commit d858f0d into prometheus:master Sep 3, 2017
@brian-brazil

Copy link
Copy Markdown
Contributor

Thanks!

@aimestereo
aimestereo deleted the fix-multiprocessing-for-celery branch September 5, 2017 12:00
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