Conversation
- Introduced new modules for error handling, HPC integration, network management, and process management. - Implemented custom exceptions for configuration and launch errors. - Added functionality to launch MAPDL on HPC clusters using SLURM scheduler. - Created data models for launch configuration, process information, and validation results. - Implemented port management functions to check availability and find free ports. - Developed validation functions to ensure proper configuration before launching MAPDL. - Enhanced process management with subprocess handling and monitoring for startup readiness.
There was a problem hiding this comment.
Pull request overview
This pull request refactors the MAPDL launcher into a modular, domain-driven architecture. The refactoring separates concerns into distinct modules: configuration resolution, validation, environment detection, network management, process handling, HPC integration, and client connection. The legacy launcher code is preserved in _launcher_legacy.py for backward compatibility.
Changes:
- Introduced modular architecture with separate modules for different launcher concerns (config, validation, environment, network, process, hpc, connection)
- Implemented immutable data models using frozen dataclasses for configuration and results
- Added comprehensive validation logic separate from configuration resolution
- Created HPC-specific module for SLURM cluster support
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ansys/mapdl/core/launcher/models.py | Defines immutable data structures for configuration, process info, validation results, and HPC job info |
| src/ansys/mapdl/core/launcher/errors.py | Introduces ConfigurationError and LaunchError custom exceptions |
| src/ansys/mapdl/core/launcher/config.py | Implements pure functions for resolving configuration from arguments, environment variables, and defaults |
| src/ansys/mapdl/core/launcher/validation.py | Provides validation functions that return ValidationResult objects without raising exceptions |
| src/ansys/mapdl/core/launcher/environment.py | Handles platform detection (WSL, Ubuntu) and environment variable preparation |
| src/ansys/mapdl/core/launcher/network.py | Manages port availability checking and free port discovery |
| src/ansys/mapdl/core/launcher/process.py | Handles subprocess creation, monitoring, and startup verification |
| src/ansys/mapdl/core/launcher/hpc.py | Provides SLURM scheduler integration for HPC cluster launches |
| src/ansys/mapdl/core/launcher/connection.py | Creates MapdlGrpc and MapdlConsole client instances |
| src/ansys/mapdl/core/launcher/init.py | Main entry point that orchestrates the launch process and re-exports legacy functions |
| src/ansys/mapdl/core/_launcher_legacy.py | Preserved legacy launcher code (renamed from launcher.py) for backward compatibility |
| @dataclass | ||
| class ValidationResult: | ||
| """Result of configuration validation. | ||
|
|
||
| Attributes: | ||
| valid: Whether configuration is valid | ||
| errors: List of error messages (prevent launch) | ||
| warnings: List of warning messages (allow launch) | ||
| """ | ||
|
|
||
| valid: bool | ||
| errors: List[str] = field(default_factory=list) | ||
| warnings: List[str] = field(default_factory=list) | ||
|
|
||
| def add_error(self, message: str) -> None: | ||
| """Add an error message and mark as invalid.""" | ||
| self.valid = False | ||
| self.errors.append(message) | ||
|
|
||
| def add_warning(self, message: str) -> None: | ||
| """Add a warning message.""" | ||
| self.warnings.append(message) | ||
|
|
There was a problem hiding this comment.
The ValidationResult class is mutable (using regular dataclass with mutable fields), but the docstring states "All models are immutable (where practical)" on line 26. Consider either making ValidationResult frozen=True and returning a new instance from add_error/add_warning methods, or clarifying in the module docstring that ValidationResult is intentionally mutable for collecting validation results.
| except ImportError as e: | ||
| # If legacy launcher doesn't exist or has errors, warn but continue | ||
| import warnings | ||
|
|
||
| warnings.warn( | ||
| f"Could not import legacy launcher functions: {e}. " | ||
| f"Some backward compatibility may be limited." | ||
| ) |
There was a problem hiding this comment.
| except ImportError as e: | |
| # If legacy launcher doesn't exist or has errors, warn but continue | |
| import warnings | |
| warnings.warn( | |
| f"Could not import legacy launcher functions: {e}. " | |
| f"Some backward compatibility may be limited." | |
| ) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
… into refactor/launcher.py
- Implement unit tests for the `launcher` module, covering orchestration, models, network, process, and validation functionalities. - Create tests for `launch_mapdl` orchestration, including connection modes, error handling, and environment preparation. - Add tests for `LaunchConfig`, `ProcessInfo`, `ValidationResult`, and `PortStatus` models to ensure proper functionality and immutability. - Develop network tests for checking port status and finding available ports. - Introduce process tests for launching, command generation, and process management. - Implement validation tests for configuration checks, resource availability, and conflicting options. - Ensure comprehensive coverage of edge cases and internal function validations.
…ove test configuration
- Improved docstrings for `check_port_status`, `find_available_port`, and related functions in `network.py` to provide clearer usage examples, parameter descriptions, and return values. - Updated docstrings in `process.py` for functions like `launch_mapdl_process`, `wait_for_process_ready`, and others to clarify their purpose, parameters, and return types. - Enhanced validation function documentation in `validation.py` to detail the checks performed, parameters, and examples for better understanding. - Added notes and examples to internal utility functions to aid developers in understanding their usage and context.
…submission handling
…rehensive command generation and subprocess management
…submission handling
… parameter handling
… of instance creation, connection, and error handling
…4422) * build: update ansys-tools-visualization-interface version constraint * chore: adding changelog file 4422.dependencies.md [dependabot-skip] --------- Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
#4424) * build: adding python 3.10 versions on dependencies to fix uv run issue * chore: adding changelog file 4424.dependencies.md [dependabot-skip] --------- Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
Description
Issue linked
NA
Checklist
draftif it is not ready to be reviewed yet.feat: adding new MAPDL command)