Hua work common#108
Merged
Merged
Conversation
Eliminates subprocess spawns for common hot paths:
- pg_sig.py: ps/os.system calls replaced with os.kill, psutil
process iteration, and subprocess.Popen for the background launcher
- pg_log.py: grep /etc/passwd -> pwd.getpwnam; os.system('more') ->
subprocess.run (avoids shell-quoting on the filename)
- pg_file.py: md5sum subprocess -> hashlib.md5 chunked read
Adds psutil as a dependency in pyproject.toml and requirements.txt.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes several OS-level interactions in rda_python_common by replacing shell command pipelines (e.g., ps, grep, md5sum, more) with Python-native implementations (psutil, hashlib, subprocess) and bumps the package version to 2.1.11.
Changes:
- Replace
ps/grep-based process discovery and child enumeration withpsutilinPgSIG. - Replace
md5sumsubprocess calls with a streaminghashlib.md5()implementation inPgFile. - Update usage display and specialist home lookup logic to avoid shelling out (
more,/etc/passwdparsing), plus dependency/version bumps.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/rda_python_common/pg_sig.py |
Switch process scanning/child management/background start to psutil + subprocess. |
src/rda_python_common/pg_log.py |
Replace os.system("more ...") with subprocess.run, and use pwd.getpwnam for specialist home/shell. |
src/rda_python_common/pg_file.py |
Implement MD5 calculation in Python via hashlib instead of md5sum. |
src/rda_python_common/__init__.py |
Bump __version__ to 2.1.11. |
requirements.txt |
Add psutil dependency. |
README.md |
Update referenced version to 2.1.11. |
pyproject.toml |
Bump project version and add psutil to dependencies. |
Comments suppressed due to low confidence (1)
src/rda_python_common/pg_sig.py:825
check_process()treats anyOSErrorfromos.kill(pid, 0)as “not running”. On POSIX,EPERMmeans the process exists but you don’t have permission to signal it, so this will incorrectly report live processes as dead. HandlePermissionError/errno.EPERMas running and only return 0 forProcessLookupError/errno.ESRCH(and possibly treat other errors conservatively/log them).
try:
os.kill(pid, 0)
except OSError:
return 0
return 1
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+232
to
+239
| if not any(aname in arg for arg in cmdline): continue | ||
| else: | ||
| exe = os.path.basename(cmdline[0]) | ||
| if exe != aname and re.sub(r'\.\w+$', '', exe) != aname: continue | ||
| results.append({ | ||
| 'pid': info['pid'], | ||
| 'ppid': info['ppid'], | ||
| 'args': ' '.join(cmdline[1:]), |
| # shell=True is required for the redirections (>> / 2>>) and trailing '&'; | ||
| # the '&' makes the shell fork the command and exit, so the command gets | ||
| # reparented to init (ppid=1), matching the lookup logic in record_background(). | ||
| subprocess.Popen(bckcmd, shell=True) |
Comment on lines
772
to
774
| else: | ||
| os.system("more " + usgname) | ||
| subprocess.run(['more', usgname]) | ||
| self.pgexit(0) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.