Skip to content
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

fix: Remove psutil dependency #3439

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

fix: Remove psutil dependency #3439

wants to merge 10 commits into from

Conversation

hpohekar
Copy link
Collaborator

@hpohekar hpohekar commented Oct 31, 2024

closes #3423

  1. Remove psutil dependency
  2. Implemented _pid_exists in replacement of psutil.pid_exists()

@hpohekar hpohekar linked an issue Oct 31, 2024 that may be closed by this pull request
@hpohekar hpohekar enabled auto-merge (squash) October 31, 2024 12:57
@mkundu1
Copy link
Contributor

mkundu1 commented Oct 31, 2024

@hpohekar I've also tested this approach. Does it work in Windows?

@hpohekar
Copy link
Collaborator Author

hpohekar commented Oct 31, 2024

@hpohekar I've also tested this approach. Does it work in Windows?

@mkundu1 Right, it was not working in Windows but we have got 2 solutions, both of them are working fine.

Personally I think 1st is better.

Solution 1:

elif platform.system() == "Windows":
        process_query_information = 0x1000
        process_handle = ctypes.windll.kernel32.OpenProcess(
            process_query_information, 0, pid
        )
        if process_handle == 0:
            return False
        else:
            ctypes.windll.kernel32.CloseHandle(process_handle)
            return True

Solution 2:

elif platform.system() == "Windows":
	try:
		command = f'tasklist /FI "PID eq {pid}"'
		result = subprocess.run(command, shell=True, text=True, capture_output=True)
		if result.returncode == 0:
			output = result.stdout.strip()
			lines = output.splitlines()
			if len(lines) == 3 and str(pid) in lines[2]:
				return True
			else:
				return False
	except Exception as e:
		return False

I have checked it locally, tests are passing in Windows now. We have also added a separate test for standalone mode.

image

@mkundu1
Copy link
Contributor

mkundu1 commented Oct 31, 2024

@hpohekar I've also tested this approach. Does it work in Windows?

@mkundu1 Right, it was not working in Windows but we have got 2 solutions, both of them are working fine.

Personally I think 1st is better.

Solution 1:

elif platform.system() == "Windows":
        process_query_information = 0x1000
        process_handle = ctypes.windll.kernel32.OpenProcess(
            process_query_information, 0, pid
        )
        if process_handle == 0:
            return False
        else:
            ctypes.windll.kernel32.CloseHandle(process_handle)
            return True

Solution 2:

elif platform.system() == "Windows":
	try:
		command = f'tasklist /FI "PID eq {pid}"'
		result = subprocess.run(command, shell=True, text=True, capture_output=True)
		if result.returncode == 0:
			output = result.stdout.strip()
			lines = output.splitlines()
			if len(lines) == 3 and str(pid) in lines[2]:
				return True
			else:
				return False
	except Exception as e:
		return False

I have checked it locally, tests are passing in Windows now. We have also added a separate test for standalone mode.

image

Tested it, looks fine. Thanks.

@mkundu1
Copy link
Contributor

mkundu1 commented Oct 31, 2024

@hpohekar watchdog_exec also uses psutil.

@prmukherj prmukherj self-requested a review October 31, 2024 18:29
@hpohekar
Copy link
Collaborator Author

hpohekar commented Nov 1, 2024

@hpohekar watchdog_exec also uses psutil.

@mkundu1 Yes. Thank you.

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.

remove psutil dependency Reduce thirdparty dependecies
3 participants