Skip to content

Conversation

@livia-macon
Copy link
Contributor

Closes #32

Comment on lines 477 to 485
objs = object.__dict__
pid = objs.get("_pid")

process = psutil.Process(pid)
process.terminate()

psutil.wait_procs(generator, timeout=10)
if process.is_running() == True:
raise Exception("Terminate process failed")

Choose a reason for hiding this comment

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

What do you think of this approach?

Suggested change
objs = object.__dict__
pid = objs.get("_pid")
process = psutil.Process(pid)
process.terminate()
psutil.wait_procs(generator, timeout=10)
if process.is_running() == True:
raise Exception("Terminate process failed")
process.terminate()
psutil.wait_procs([process], timeout=10)
if process.is_running():
raise Exception("Terminate process failed")

Makes code easier and doesn't do duplicate searches.

@kayqueGovetri kayqueGovetri self-requested a review March 21, 2023 18:09
Copy link
Collaborator

@hhslepicka hhslepicka left a comment

Choose a reason for hiding this comment

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

Great work so far! Just an improvement suggestion for the find_process and a change request for the terminate_process.

Comment on lines 469 to 485
def terminate_process(self, generator: Generator):
"""
Terminate the process via the received generator object.
Args:
generator (Generator): The Generator Object.
"""
for object in generator:
objs = object.__dict__
pid = objs.get("_pid")

process = psutil.Process(pid)
process.terminate()

psutil.wait_procs(generator, timeout=10)
if process.is_running() == True:
raise Exception("Terminate process failed")
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this method, we should take care of a single process and not all of them.

Suggested change
def terminate_process(self, generator: Generator):
"""
Terminate the process via the received generator object.
Args:
generator (Generator): The Generator Object.
"""
for object in generator:
objs = object.__dict__
pid = objs.get("_pid")
process = psutil.Process(pid)
process.terminate()
psutil.wait_procs(generator, timeout=10)
if process.is_running() == True:
raise Exception("Terminate process failed")
def terminate_process(self, process: Process):
"""
Terminate the process via the received generator object.
Args:
process (Process): The process to terminate.
"""
process.terminate()
process.wait(10)
if process.is_running():
raise Exception("Terminate process failed")

Comment on lines 462 to 463
if (name is not None and process.name() == name) or \
(pid is not None and process.pid == pid):
Copy link
Collaborator

Choose a reason for hiding this comment

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

For name we could do a partial match checking if name is part of process.name().

Here is a suggestion:

Suggested change
if (name is not None and process.name() == name) or \
(pid is not None and process.pid == pid):
if (name is not None and name in process.name()) or \
(pid is not None and process.pid == pid):

This will allow us to search for a process without knowing its full name or have an exact match.

@hhslepicka hhslepicka merged commit 60a687b into main Mar 22, 2023
@hhslepicka hhslepicka deleted the ENH/adds-methods-to-find-and-terminate-process branch March 22, 2023 19:45
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.

Add find_process method

4 participants