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.

Sign up for GitHub

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

Jump to bottom

Do not delete pid file during restart #1116

Merged
merged 2 commits into from
Mar 2, 2020
Merged

Do not delete pid file during restart #1116

merged 2 commits into from
Mar 2, 2020

Conversation

biozz
Copy link
Contributor

@biozz biozz commented Feb 29, 2020

  • the pid file should be remove just before exiting

Fixes #1080

@biozz
Do not delete pid file during restart
c2003b6 
- the pid file should be remove just before exiting

Fixes !1080
@coveralls
Copy link

coveralls commented Feb 29, 2020 •
edited
Loading

Coverage Status

Coverage remained the same at ?% when pulling 7391f6d on biozz:do-not-delete-pid-file-during-restart into e509ee5 on circus-tent:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling c2003b6 on biozz:do-not-delete-pid-file-during-restart into e509ee5 on circus-tent:master.

k4nar
k4nar reviewed Mar 1, 2020
View reviewed changes
circus/circusd.py Outdated
# Do not delete pid file during restart loop
# i.e. delete it only when exiting
if pidfile is not None:
pidfile.unlink()
Copy link
Contributor

Choose a reason for hiding this comment

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

The PID file won't be erased if an exception is raised in the loop. I think it should, as was before the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think I get it now. It will stay in finally block but I will also check for restart is False, because that what was the issue all about.

@k4nar
Copy link
Contributor

k4nar commented Mar 2, 2020

Thank you 👍 . Like for #1115, I'll merge this even though the tests are red. This is an unrelated error that will be fixed in another PR.

@k4nar k4nar merged commit fcb57d1 into circus-tent:master Mar 2, 2020
@biozz biozz added this to the 0.17 milestone Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k4nar k4nar k4nar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.17
Development

Successfully merging this pull request may close these issues.

bug: circusd removes pidfile on arbiter restart
3 participants
@biozz @coveralls @k4nar