Reconcile legacy Pg*.py modules with the pg_*.py classes (3.0.14) - #122
Merged
Merged
Conversation
…version to 3.0.14 The module-level Pg*.py twins had drifted years behind the active class-based pg_*.py implementations, so downstream callers still importing the old names were silently running stale (and in places broken) logic. All nine legacy modules are now AST-equivalent to their active counterparts, which also picks up the accumulated bug fixes (Globus task waits, dssgrp-only user lookup, setuid chmod, psutil process scans, hashlib md5) and drops the retired HPSS, SLURM and UCAR People DB code paths. open_output/OUTPUT deliberately stays in PgOPT.py, since ten downstream files reference PgOPT.OUTPUT and it needs PGOPT['extlog']; it now also assigns PgLOG.OUTPUT so the ported PgLOG.pgexit() can still close it. Also fixes two defects that were present in both trees: endtime() split 'HH:MM:SS' on a literal 'T' and raised IndexError, and tosystem() declared logact=0 while the body kept the "if logact is None" idiom, so the intended LOGWRN default was dead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed runtime and security issues in the changed code paths (e.g., a bad open_output call site, hard-coded token default, and incorrect process-liveness handling on PermissionError).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR reconciles multiple legacy Pg*.py modules with their newer class-based pg_*.py implementations so that downstream code importing legacy names gets equivalent behavior, and bumps the package version to 3.0.14.
Changes:
- Aligns legacy modules’ logic with the active class-based implementations (including multiple bug fixes and removals of retired paths like SLURM/HPSS).
- Modernizes process scanning and background/timeout handling (moving from
ps | greptopsutiland improving fork/timeout behavior). - Updates file operations/utilities (Globus wait behavior, in-Python MD5, sorting/search helpers) and bumps version strings across packaging/docs.
File summaries
| File | Description |
|---|---|
| src/rda_python_common/PgUtil.py | Utility refactors and bug fixes (datetime parsing, sorting/search helpers, text-file detection). |
| src/rda_python_common/PgSplit.py | Fixes dict/list construction pitfalls and normalizes optional logact defaults. |
| src/rda_python_common/PgSIG.py | Replaces `ps |
| src/rda_python_common/PgOPT.py | Keeps open_output in PgOPT, syncs PgLOG.OUTPUT, updates user UID lookup via PgDBI. |
| src/rda_python_common/PgLOG.py | Drops retired HPSS/SLURM config, improves logging/email mechanics, adds command path expansion and output closing. |
| src/rda_python_common/PgLock.py | Tightens lock info formatting and partition lock counter updates. |
| src/rda_python_common/PgFile.py | Adjusts Globus copy semantics (wait for task completion), switches MD5 to hashlib, improves object-store stat handling. |
| src/rda_python_common/PgDBI.py | Removes retired DB aliases, improves error parsing robustness, migrates user lookup to dssgrp and “missed user” handling. |
| src/rda_python_common/PgCMD.py | Updates dscheck init/validation and enhances dynamic option retrieval (incl. optional ssh + command path expansion). |
| src/rda_python_common/pg_util.py | Active-tree fix mirrored: endtime() uses non-digit splitting (avoids IndexError). |
| src/rda_python_common/pg_log.py | Active-tree update mirrored: tosystem() default logact becomes None → LOGWRN. |
| src/rda_python_common/init.py | Bumps __version__ to 3.0.14. |
| README.md | Updates displayed current version to 3.0.14. |
| pyproject.toml | Bumps package version to 3.0.14. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
1216
to
+1220
| if valid_command(sm): SETPGLOG("EMLSEND", f"{sm} -t") # send email command | ||
| SETPGLOG("DBGLEVEL", '') # debug level | ||
| SETPGLOG("BAOTOKEN", 's.lh2t2kDjrqs3V8y2BU2zOocT') # OpenBao token | ||
| SETPGLOG("DBGPATH", PGLOG['DSSDBHM']+"/log") # path to debug log file | ||
| SETPGLOG("OBJCTBKT", "gdex-data") # default Bucket on Object Store | ||
| SETPGLOG("BACKUPEP", "gdex-quasar") # default Globus Endpoint on Quasar | ||
| SETPGLOG("DRDATAEP", "gdex-quasar-drdata") # DRDATA Globus Endpoint on Quasar | ||
| SETPGLOG("BAOTOKEN", 's.MdOPGayn0HcuuSPrmMqCvzJA') # OpenBao token | ||
| SETPGLOG("DBGPATH", PGLOG['DSSDBHM']+"/log") # path to debug log file | ||
| SETPGLOG("OBJCTBKT", "gdex-data") # default Bucket on Object Store |
Comment on lines
+567
to
571
| uid = PgDBI.get_user_uid(params['LN']) | ||
| if not uid: PgLOG.pglog("Could not get user.uid for " + params['LN'], PGOPT['extlog']) | ||
| PGOPT['UID'] = uid | ||
| PgLOG.open_output(params['OF'] if 'OF' in params else None) | ||
|
|
Comment on lines
667
to
+672
| def check_process(pid): | ||
|
|
||
| buf = PgLOG.pgsystem("ps -p {} -o pid".format(pid), PgLOG.LGWNEX, 20) | ||
| if buf: | ||
| mp = r'^\s*{}$'.format(pid) | ||
| lines = buf.split('\n') | ||
| for line in lines: | ||
| if re.match(mp, line): return 1 | ||
|
|
||
| return 0 | ||
| try: | ||
| os.kill(pid, 0) | ||
| except OSError: | ||
| return 0 | ||
| return 1 |
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.
Summary
Pg*.pytwins with their active class-basedpg_*.pyimplementations, which had drifted years behind. All nine are now AST-equivalent to their active counterparts, so downstream code still importing the old names gets the same logic.dssdb.dssgrp-only user lookup (retired UCAR People DB), setuidchmod,psutilprocess scans replacingps | grep,hashlibmd5. Drops the retired HPSS and SLURM code paths and theirPGLOGkeys.endtime()split'HH:MM:SS'on a literal'T'and raisedIndexError;tosystem()declaredlogact=0while the body kept theif logact is Noneidiom, so the intendedLOGWRNdefault was dead.3.0.14inpyproject.toml,__init__.pyandREADME.md.Deliberate deviation
open_output/OUTPUTstays inPgOPT.pyrather than moving toPgLOGalongside the active tree: ten downstream files referencePgOPT.OUTPUT, and the function needsPGOPT['extlog'], so the move would be circular. It now also assignsPgLOG.OUTPUTso the portedPgLOG.pgexit()can still close it.Test plan
addNoLeapDate/is_leapyearaliases and the documentedopen_outputdeviation)globaldeclarations and no missing imports (checked mechanically per file)py_compileclean on all 9 legacy modules plus the two touched active modulesrda_python_dsarchimports against the reconciled packageendtime('01:02:03','H')->01:59:59,endtime('01:02:03','N')->01:02:59,endtime('','H')->00:59:59;PgLOG.tosystemdefaultlogactisNone;PgOPT.OUTPUTandPgLOG.OUTPUTboth resolve