Skip to content
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

Implementing unique session id #1912

Merged
merged 62 commits into from
Jul 17, 2023
Merged

Implementing unique session id #1912

merged 62 commits into from
Jul 17, 2023

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Mar 13, 2023

It should improve stability. Let's see.

Current situation

CICD fails because the docker image crashes quite frequently. We automatically reboot it (--restart always docker flag), but then the image comes with no data, clean.

But PyMAPDL is not aware that the database has been cleaned hence it request data which gives errors of different kind.

Approach

In this PR, I have implemented a session id unique for each PyMAPDL connection. Everytime it connects to a MAPDL session, using _connect method, it will fix a session ID. Also, when we issue mapdl.clear.

Every time we issue mapdl.run we check that the ids are the same in the client and in the server (only at testing). If they are not, I raise an issue which is catched by pytests, and re-run the test completely.

Opinion

I think the idea is good. But ideally we want to implement something that checks the health of the gRPC channel.
So far, I'm going to test this, and consider late:

  • Use gRPC methods/functions for checking the channel health
  • Expand id check to vgets.

Close #1915

@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc Maintenance General maintenance of the repo (libraries, cicd, etc) Enhancement Improve any current implemented feature New Feature Request or proposal for a new feature labels Mar 13, 2023
@github-actions
Copy link
Contributor

Please add one of the following labels to add this contribution to the Release Notes 👇

@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #1912 (734c1fa) into main (997e288) will increase coverage by 6.28%.
The diff coverage is 97.34%.

@@            Coverage Diff             @@
##             main    #1912      +/-   ##
==========================================
+ Coverage   80.82%   87.10%   +6.28%     
==========================================
  Files          44       45       +1     
  Lines        7489     8052     +563     
==========================================
+ Hits         6053     7014     +961     
+ Misses       1436     1038     -398     

src/ansys/mapdl/core/mapdl_grpc.py Show resolved Hide resolved
@germa89 germa89 marked this pull request as ready for review July 3, 2023 15:40
tests/conftest.py Outdated Show resolved Hide resolved
@germa89 germa89 enabled auto-merge (squash) July 17, 2023 10:38
@germa89 germa89 merged commit 4ba1ca9 into main Jul 17, 2023
22 checks passed
@germa89 germa89 deleted the feat/unique_session_id branch July 17, 2023 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Related with CICD, Github Actions, etc DO NOT MERGE Not ready to be merged yet Enhancement Improve any current implemented feature Maintenance General maintenance of the repo (libraries, cicd, etc) New Feature Request or proposal for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CICD stability
3 participants