sals.process cleanup/refactor

[sameh-farouk]

Description

sals.process cleanup/refactor

Changes

• Fixing related issues and bugs sal process needs some cleanup #467, sals.process.kill function is not efficient and returns True even the Process still a live #557, sals.process.get_process_by_port not distinguish between ipv4 and ipv6 connections, or between TCP and UDP ports #561.
• Refactoring and enhancing the module.
• Re-implementing some functions, removing the ps shell command completely, and depend on psutil instead.
• Many functions enriched with new args, and become more generic
• Ensuring complete docstring and matching google style.
• Adding proper error handling.
• Adding proper logging.
• Remove redundant code, unused import, etc.
• Adding a new function kill_proc_tree() to kill all subprocesses that could have been spawn from a given process.
• Adding a new function kill_all_pids() to kill all processes in a given list of PIDs
• Adding more examples in the module docstring.
• improving the unit test, fixing few bugs.
• regenerate docs

Related Issues

closes #467
#541
closes #557
closes #561

Decisions Needed:

Suggested changes below are not included in this PR for compatibility reasons with any old code.
but it may be considered, discussed, and make decisions about:

• 1: kill(), check_start() and check_stop() functions return True when succeded. raise an exception otherwise.

  js-ng/jumpscale/sals/process/__init__.py

  Line 102 in e610686

  def kill(proc, sig=signal.SIGTERM, timeout=5, sure_kill=False):

  
  

  js-ng/jumpscale/sals/process/__init__.py

  Line 271 in e610686

  def check_start(cmd, filterstr, n_instances=1, retry=1):

  
  

  js-ng/jumpscale/sals/process/__init__.py

  Line 313 in e610686

  def check_stop(cmd, filterstr, retry=1, n_instances=0):

  • the returned True is redundant and not needed, it actually never return False. in case of the operation failed we raise an exception.

  Decision: the function should return None and the calling code should call the function inside try .. except, and never check for the output.

  Update: Solved, after discussion with @abom. now, these functions return None.

• 2: kill_all() function default signal is SIGKILL and inconsistent with kill() function that have SIGTERM as a default signal also it almost have same functionality as kill_process_by_name() function.

  js-ng/jumpscale/sals/process/__init__.py

  Line 167 in e610686

  def kill_all(process_name, sig=signal.SIGKILL):

  Decision: kill_all() default signal should be re-considered also the function itself should be deprecated and removed in favor of kill_process_by_name() because they are almost the same.

  Update: Solved, after discussion with @abom. now, this function deprecated and removed in favor of kill_process_by_name()

• 3: the module functions divided between functions that return the processes as a list[int] represents the processes ID(s) (due to it was initially parsing the PIDs from the ps shell command output), and functions that return the processes as psutil.Process objects. passing PIDs is not reliable in case the process is gone and its PID reused by another process, also this will affect the performance when we initiate the psutil.Process objects repeatedly from pids between function calls.

  • ex: we iterate through some psutil.Process objects in one function to do some work, when returning the function getting processes PIDs, appended it to a list, and send it to another function. the second function will again initiate a Process object to do its work. this affects the performance and may end with dealing with a different process than the intended one.

  Decision: as we now depend completely on psutil in this module, all functions should return psutil.Porcess objects instead of PIDs: list[int], another alternative, the affected functions - specially get_pids() - could take a parameter to decide whether it should return processes IDs or the Processes objects.

• 4: is_port_listening() function is irrelevant in the process sal. it is already implemented in the nettools sal and should not re-implemented here.

  js-ng/jumpscale/sals/process/__init__.py

  Line 578 in e610686

  def is_port_listening(port, ipv6=False):

  Decision: the function should be deprecated and removed in favor of tcp_connection_test() from the nettools sal.

Checklist

• Pre-commit hook is installed to do formatting checks before committing code...etc
• Tests included
• Build pass
• Documentation
• Code format and docstrings