Skip to content

Refactor subprocess calls to use run_command utility #19

Description

@gregorweiss

Background

PR #11 introduced run_command() in mdfactory/utils/utilities.py - a unified interface for subprocess execution with graceful failure mode, timeout support, and consistent error handling. Cluster autodiscovery has been migrated to use it.

Several other subprocess calls could benefit from this utility for consistency and maintainability.

Locations

mdfactory/cli.py:1450 - Simulation runner

Redirects output to file handles. Could use run_command() with capture_output=False for consistency, though functionally equivalent.

mdfactory/cli.py:1812 - Config editor

subprocess.run([editor, str(config_path)], check=False)

Interactive command - consider leaving as-is or migrate for consistency.

mdfactory/utils/sync_config.py:142 - Foundry validation

if shutil.which("fdt") is None:
    logger.error("\`fdt\` command not found...")
    sys.exit(1)
result = subprocess.run(["fdt", "config"], check=False)
if result.returncode != 0:
    logger.error("\`fdt config\` failed...")
    sys.exit(1)

Could simplify with graceful=False mode - combines binary check and execution.

mdfactory/analysis/bilayer/artifacts/__init__.py - VMD/ffmpeg

Three calls using check=True for critical rendering operations (lines 56, 113, 135). Straightforward migration.

Benefits

  • Single subprocess pattern across codebase
  • Easy to add timeouts/logging globally if needed later
  • Simpler mocking in tests (one utility vs subprocess everywhere)
  • Consistent error handling

Non-Goals

  • No behavior changes
  • Interactive commands can stay as-is if preferred
  • Keep existing check=True semantics where used

Tasks

  • Evaluate each location
  • Migrate where appropriate
  • Verify tests pass

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions