-
Notifications
You must be signed in to change notification settings - Fork 142
fix: open gui not working #4270
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
Conversation
- Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks.
…mport, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the open_gui method by ensuring environment variables are properly propagated and preventing circular import issues.
Key Changes:
- Environment variables are now added to start parameters in
launch_mapdl - Import path changed to use package-level
get_mapdl_pathto avoid circular imports MapdlGrpc._launchsignature updated to accept optional start parameters
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ansys/mapdl/core/launcher.py | Adds env_vars to start_parm dictionary to preserve environment variables |
| src/ansys/mapdl/core/mapdl_grpc.py | Updates _launch method signature to accept optional start_parm and removes explicit port parameter from _connect call |
| src/ansys/mapdl/core/mapdl_core.py | Adds exec_file parameter to open_gui, replaces import to avoid circular dependency, and adds error handling for missing executable path |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4270 +/- ##
==========================================
+ Coverage 91.05% 91.37% +0.31%
==========================================
Files 193 193
Lines 15720 15721 +1
==========================================
+ Hits 14314 14365 +51
+ Misses 1406 1356 -50 🚀 New features to boost your workflow:
|
…apdlGrpc._launch and open_gui flow Add unit tests to verify: - env_vars are propagated into start_parm when launching MAPDL - SLURM-specific bootstrap vars are injected when launching on HPC (replace_env_vars) - MapdlGrpc._launch prefers a provided start_parm over the instance _start_parm - open_gui invokes the correct executable and flags (mocking subprocess.call) - complete open_gui flow calls expected lifecycle methods and reconnects via _launch These tests exercise env propagation, command building, and reconnection behavior for launcher and gRPC flows.
- add MagicMock and Mock imports used across tests - update test_env_vars_with_slurm_bootstrap to call launch_mapdl(...) with replace_env_vars and remove unused assignment - remove duplicate/unnecessary imports in tests - correctly bind MapdlGrpc._launch when testing provided start_parm - simplify exception handlers (avoid unused exception variable) - add assertion to ensure _cache_routine() is called during open_gui reconnection flow
…mapdl lifecycle methods and using ExitStack to manage patches
… call - remove @requires("local") from open_gui tests so they don't depend on local env - mock mapdl._local, pathlib.Path.is_file and ansys.mapdl.core.mapdl_core.call (instead of subprocess.call) - use ExitStack / nested context managers to manage patches and keep test isolation - ensure original mapdl._launch is restored after the test
Add # nosec B703 to broad except handlers in tests to silence bandit warnings. Remove redundant MagicMock imports and eliminate unused "run_location" keys from test payloads to clean up and simplify test_launcher.py.
|
@pyansys-ci-bot LGTM. |
pyansys-ci-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* feat(core): add exec_file param to open_gui and improve execable lookup - Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks. * fix(core): propagate env_vars into start parameters, avoid circular import, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.
* feat(core): add exec_file param to open_gui and improve execable lookup - Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks. * fix(core): propagate env_vars into start parameters, avoid circular import, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.
* feat(core): add exec_file param to open_gui and improve execable lookup - Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks. * fix(core): propagate env_vars into start parameters, avoid circular import, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.
* feat(core): add exec_file param to open_gui and improve execable lookup - Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks. * fix(core): propagate env_vars into start parameters, avoid circular import, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.
* feat(core): add exec_file param to open_gui and improve execable lookup - Add typed exec_file parameter to open_gui and document its behavior. - Defer importing get_mapdl_path and only attempt to resolve the MAPDL executable when ansys.mapdl.core ATP helper is available. - Use start_parm exec_file if provided, otherwise try get_mapdl_path; raise a clear MapdlRuntimeError if no executable path can be determined. - Minor docstring/format tweaks. * fix(core): propagate env_vars into start parameters, avoid circular import, and make MapdlGrpc._launch accept start_parm - Add env_vars to start_parm in launch_mapdl so environment vars are carried in generated start parameters. - Replace import from ansys.mapdl.core.launcher with package-level get_mapdl_path to prevent circular imports. - Update MapdlGrpc._launch signature to accept an optional start_parm, use it when provided, and call _connect() without passing the port explicitly.

Description
Fix
open_guimethod.Issue linked
Close #3993
Close #4269
Checklist
draftif it is not ready to be reviewed yet.feat: adding new MAPDL command)