Skip to content

use pg-ctl stop to cleanup before killing the process #16

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

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

use pg-ctl stop to cleanup before killing the process #16

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pg_temp/pg_temp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Set up a temporary postgres DB"""

import itertools
import os
import sys
Expand Down Expand Up @@ -358,6 +359,8 @@ def cleanup(self):
stdout=subprocess.DEVNULL,
)
elif self.pg_process:
pg_stop_cmd = ['pg_ctl', 'stop', '-D', self.pg_data_dir, '-m' 'fast']
Copy link
Owner

Choose a reason for hiding this comment

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

We need to be able to override pg_ctl in the same way that we do postgres or initdb, in the case the user is using an alternative postgres install that is not in the PATH. This needs a default, and an argument (plus said default) in the constructor. See for example DEFAULT_POSTGRES or DEFAULT_INITDB

self.run_cmd(pg_stop_cmd, level=2, bg=True).wait()
Copy link
Owner

Choose a reason for hiding this comment

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

I think since run_cmd with level 2 can potentially produce stdout and stderr, instead of .wait() we should call .communicate(). Also, since this command can be caught by the pg process or hang, we should have a timeout at which point we give up and call kill() anyway. Maybe 10 seconds?
The python docs have an example of how to do this properly, catching the TimeoutExpired exception https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate

self.pg_process.kill()
self.pg_process.wait()
for d in [self.pg_temp_dir]:
Expand Down