-
Notifications
You must be signed in to change notification settings - Fork 950
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
bugfix: add judge for whether pidfile path is given #1374
bugfix: add judge for whether pidfile path is given #1374
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1374 +/- ##
=======================================
Coverage 17.29% 17.29%
=======================================
Files 190 190
Lines 11913 11913
=======================================
Hits 2060 2060
Misses 9706 9706
Partials 147 147 |
@Ace-Tang please create PR base on branch |
Signed-off-by: Ace-Tang <aceapril@126.com>
defer func() { | ||
if err := os.Remove(cfg.Pidfile); err != nil { | ||
logrus.Errorf("failed to delete pidfile: %s", err) | ||
if cfg.Pidfile != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Pidfile exists, do we need to truncate it's content ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at utils.NewPidfile
, it won't go ahead when pidfile has value, it a common logic most process deal with.
LGTM |
Signed-off-by: Ace-Tang aceapril@126.com
Ⅰ. Describe what this PR did
save pid into pidfile only when pidfile path is given, make code more reasonable.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews