-
Notifications
You must be signed in to change notification settings - Fork 23
ENH: Adds the find_process method and terminate_process method #33
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
ENH: Adds the find_process method and terminate_process method #33
Conversation
botcity/core/bot.py
Outdated
| 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") |
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.
What do you think of this approach?
| 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.
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.
Great work so far! Just an improvement suggestion for the find_process and a change request for the terminate_process.
botcity/core/bot.py
Outdated
| 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") |
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.
For this method, we should take care of a single process and not all of them.
| 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") |
botcity/core/bot.py
Outdated
| if (name is not None and process.name() == name) or \ | ||
| (pid is not None and process.pid == pid): |
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.
For name we could do a partial match checking if name is part of process.name().
Here is a suggestion:
| 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.
Closes #32