feat: using entrypoint to start MAPDL - #4098
Merged
Merged
Conversation
Contributor
|
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
Contributor
Reviewer's GuideThis PR refactors the container startup by introducing a dedicated entrypoint script that initializes MAPDL and conditionally launches the DPF server, augments CI workflows with DPF support and enhanced logging, and updates the core init to detect ansys.dpf and matplotlib. Sequence diagram for container startup with entrypoint and conditional DPF server launchsequenceDiagram
participant CI as CI Workflow
participant Docker as Docker Container
participant Entrypoint as Entrypoint Script
participant MAPDL as MAPDL Process
participant DPF as DPF Server
CI->>Docker: Start container
Docker->>Entrypoint: Run /entrypoint.sh
Entrypoint->>Entrypoint: Check ANSYS_DPF_ACCEPT_LA or RUN_DPF_SERVER
alt DPF server enabled
Entrypoint->>DPF: Start DPF server
end
Entrypoint->>MAPDL: Start MAPDL process
Class diagram for updated core init module dependency detectionclassDiagram
class __init__ {
+_HAS_ATP: bool
+_HAS_CLICK: bool
+_HAS_DPF: bool
+_HAS_MATPLOTLIB: bool
+_HAS_PANDAS: bool
+_HAS_PIM: bool
+_HAS_PYANSYS_REPORT: bool
+_HAS_PYVISTA: bool
+_HAS_REQUESTS: bool
+_HAS_TQDM: bool
+_HAS_VISUALIZER: bool
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @germa89 - I've reviewed your changes - here's some feedback:
- The CI workflows use both DPF_START_SERVER and RUN_DPF_SERVER—unify this environment variable across all scripts to avoid startup mismatches.
- The entrypoint.sh still has hard-coded ports and a commented-out debug directory; consider parameterizing DPF_PORT_INTERNAL and enabling DATAPROCESSING_DEBUG directory creation for consistency.
- You removed quotes around P_SCHEMA in the Docker arguments—keep variables quoted to prevent any shell word-splitting or glob-expansion issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CI workflows use both DPF_START_SERVER and RUN_DPF_SERVER—unify this environment variable across all scripts to avoid startup mismatches.
- The entrypoint.sh still has hard-coded ports and a commented-out debug directory; consider parameterizing DPF_PORT_INTERNAL and enabling DATAPROCESSING_DEBUG directory creation for consistency.
- You removed quotes around P_SCHEMA in the Docker arguments—keep variables quoted to prevent any shell word-splitting or glob-expansion issues.
## Individual Comments
### Comment 1
<location> `.ci/entrypoint.sh:33` </location>
<code_context>
+echo "Starting MAPDL..."
+echo "Using executable path: ${EXEC_PATH}"
+
+$EXEC_PATH -grpc -dir /jobs -"${DISTRIBUTED_MODE}" -np 2 -db -6000 -m -6000 -
\ No newline at end of file
</code_context>
<issue_to_address>
Quoting of DISTRIBUTED_MODE may result in an invalid command-line argument.
Using quotes here causes the argument to be passed as -"dmp" instead of -dmp, which may not be recognized. Remove the quotes to ensure correct argument parsing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
github-actions
Bot
added
CI/CD
Contributor
There was a problem hiding this comment.
Pull Request Overview
Introduces a dedicated entrypoint script for launching MAPDL containers with optional DPF server support and updates CI workflows to expose new ports, control flags, and collect DPF logs.
- Add
.ci/entrypoint.shto centralize container startup logic and conditional DPF server launch. - Refactor
start_mapdl.shto delegate execution to the new entrypoint and pass through environment variables for ports and modes. - Update GitHub Actions workflows to set
DPF_PORT_INTERNAL, toggleRUN_DPF_SERVER, adjustpytestparameters, and collect DPF logs.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ansys/mapdl/core/init.py | Add _HAS_DPF flag and reorder feature-detection variables. |
| .github/workflows/test-remote.yml | Pass DPF_PORT_INTERNAL, control RUN_DPF_SERVER, update pytest flags, remove manual DPF start block. |
| .github/workflows/test-local.yml | Add DATAPROCESSING_DEBUG environment variable for DPF logs location. |
| .ci/start_mapdl.sh | Update Docker run flags (DPF_PORT_ARG, ANSYS_DPF_ACCEPT_LA, entrypoint mount) and export new envs. |
| .ci/entrypoint.sh | New entrypoint script to conditionally start the DPF server and then launch MAPDL. |
| .ci/collect_mapdl_logs_remote.sh | Copy /home/mapdl/dpf_logs directory from container into CI artifacts. |
| .ci/collect_mapdl_logs_locals.sh | Move /home/mapdl/dpf_logs into local log output directory. |
Comments suppressed due to low confidence (2)
.github/workflows/test-remote.yml:268
- The upload-artifact step has no "if: always()" condition, so artifacts may not be uploaded on failure. Consider adding
if: always()to ensure logs are collected even if earlier steps fail.
- name: "Upload pytest reports to GitHub"
.github/workflows/test-local.yml:126
- The
DATAPROCESSING_DEBUGvariable is set but never referenced elsewhere in the workflow or scripts. Confirm whether it’s needed or remove it.
DATAPROCESSING_DEBUG: /home/mapdl/dpf_logs
germa89
enabled auto-merge (squash)
July 14, 2025 10:20
Collaborator
Author
|
@pyansys-ci-bot LGTM. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4098 +/- ##
==========================================
- Coverage 91.82% 91.81% -0.01%
==========================================
Files 187 187
Lines 15033 15034 +1
==========================================
Hits 13804 13804
- Misses 1229 1230 +1 🚀 New features to boost your workflow:
|
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.
Description
As the title. Additionally, enhance logging scripts and add DPF server support in CI workflows.
Issue linked
NA but related to #1300.
Checklist
draftif it is not ready to be reviewed yet.feat: adding new MAPDL command)Summary by Sourcery
Introduce a container entrypoint for launching MAPDL with optional DPF server support and update CI workflows and logging to integrate DPF, while adding DPF dependency detection in the Python package initialization.
New Features:
Enhancements:
CI: