-
Notifications
You must be signed in to change notification settings - Fork 120
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
fix: Update the tox.ini to different tox env #3164
base: main
Are you sure you want to change the base?
Conversation
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3164 +/- ##
==========================================
+ Coverage 82.36% 82.40% +0.03%
==========================================
Files 57 58 +1
Lines 10004 10006 +2
==========================================
+ Hits 8240 8245 +5
+ Misses 1764 1761 -3 |
a4fd62c
to
1c61a30
Compare
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.
This is going in the right direction, thanks @Revathyvenugopal162.
Co-authored-by: Jorge Martínez <[email protected]>
…to fix/improve-tox
bc25169
to
2a40945
Compare
docker run \ | ||
--env ANSYSLMD_LICENSE_FILE={env:ANSYSLMD_LICENSE_FILE} \ | ||
--name mapdl -p {env:LOCAL_MAPDL_PORT}:50052 \ |
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.
Thanks for the heads up, @germa89.
We should take advantage of this and add a rule for building a docker image locally. This will help us to review the docker/
directory and the current image building process.
This directory could look like this:
docker/
|- linux/
|- Dockerfile
|- docker-compose.yml
|- distributions/
|- .gitignore # Ignore everything except the dir itself
|- .gitkeep
Users should just copy MAPDL artifacts in the distributions/
dir. Then, running:
tox -e docker-build-images
would trigger the docker-compose.yml
file.
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.
Building the docker image is a bit complex.. because the dockerignore
depends on the MAPDL version.
The docker
directory is not used by the CICD nor myself to build the docker image at the moment, hence it is not fully maintained.
We have some instructions about building the docker image in: https://mapdl.docs.pyansys.com/version/stable/getting_started/make_container.html
We could meet next week to discuss this more in details.
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
985f42f
to
58aaebb
Compare
…to fix/improve-tox
@jorgepiloto @Revathyvenugopal162 how are we doing with this? :) |
Sorry for the delay, I was tied up with the theme reformatting. I’ll dive into this PR next week, fix the issue, and keep you updated on the progress. |
Hello! 👋 Your PR is changing the image cache. So I am attaching the new image cache in a new commit. This commit does not re-run the CICD workflows (since no changes are made in the codebase) therefore you will see the actions showing in their status You might want to rerun the test to make sure that everything is passing. You can retrigger the CICD sending an empty commit You will see this message everytime your commit changes the image cache but you are not attaching the updated cache. 🤓 |
…to fix/improve-tox
No description provided.