Skip to content

Conversation

@germa89
Copy link
Collaborator

@germa89 germa89 commented Oct 27, 2025

Description

  • Add _can_access_process() and use it in stop() to skip processes not owned by the
    current user or that raise AccessDenied/NoSuchProcess when queried.
  • Broaden exception handling in stop() to catch psutil.AccessDenied where appropriate.
  • Make is_ansys_process() resilient to AccessDenied/NoSuchProcess and return False when
    process info cannot be accessed.
  • Add unit test (test_pymapdl_stop_permission_handling) and adjust mocks to include
    username behavior to ensure permission edge-cases are handled without crashing.

Issue linked

Close #4256

Checklist

… when stopping MAPDL

- Add _can_access_process() and use it in stop() to skip processes not owned by the
  current user or that raise AccessDenied/NoSuchProcess when queried.
- Broaden exception handling in stop() to catch psutil.AccessDenied where appropriate.
- Make is_ansys_process() resilient to AccessDenied/NoSuchProcess and return False when
  process info cannot be accessed.
- Add unit test (test_pymapdl_stop_permission_handling) and adjust mocks to include
  username behavior to ensure permission edge-cases are handled without crashing.
Copilot AI review requested due to automatic review settings October 27, 2025 18:04
@germa89 germa89 requested a review from a team as a code owner October 27, 2025 18:04
Copy link
Contributor

Copilot AI left a 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 a critical bug where the pymapdl stop command would crash when encountering processes owned by other users or when access is denied to process information. The fix adds permission checking and proper exception handling to gracefully skip inaccessible processes.

Key changes:

  • Added _can_access_process() helper function to verify process ownership before attempting operations
  • Enhanced exception handling throughout to catch psutil.AccessDenied alongside psutil.NoSuchProcess
  • Made is_ansys_process() resilient to permission errors by returning False when process info is inaccessible

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/ansys/mapdl/core/cli/stop.py Added _can_access_process() function and improved exception handling to skip processes owned by other users
src/ansys/mapdl/core/launcher.py Wrapped is_ansys_process() logic in try-except to handle AccessDenied and NoSuchProcess exceptions
tests/test_cli.py Added comprehensive test for permission handling scenarios and updated mock processes to include username behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added the bug Issue, problem or error in PyMAPDL label Oct 27, 2025
@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.37%. Comparing base (566d59e) to head (a25aa7d).
⚠️ Report is 1 commits behind head on main.

❌ Your patch status has failed because the patch coverage (75.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4281      +/-   ##
==========================================
+ Coverage   91.19%   91.37%   +0.18%     
==========================================
  Files         193      193              
  Lines       15726    15740      +14     
==========================================
+ Hits        14341    14383      +42     
+ Misses       1385     1357      -28     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@germa89
Copy link
Collaborator Author

germa89 commented Oct 28, 2025

Skipping coverage as there aren't that many lines.

@pyansys-ci-bot LGTM.

@germa89 germa89 enabled auto-merge (squash) October 28, 2025 11:42
Copy link
Contributor

@pyansys-ci-bot pyansys-ci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approving this PR because germa89 said so in here 😬

LGTM

@germa89 germa89 merged commit c7bc797 into main Oct 28, 2025
76 of 86 checks passed
@germa89 germa89 deleted the fix/pymapdl-stop-cli branch October 28, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue, problem or error in PyMAPDL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pymapdl stop --all attempts to kill other users instances

3 participants