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

Improved ssh retry mechanism #501

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Improved ssh retry mechanism #501

wants to merge 16 commits into from

Conversation

LanderOtto
Copy link
Collaborator

@LanderOtto LanderOtto commented Jul 17, 2024

This commit improves the retry mechanism to be more resilent. Before this commit, the mechanism involves only the connection opening. Now, it covers both connection opening and connection communication exceptions.

Moreover, the close method of the SSHContext class was fixed calling the asynchronous wait_closed method of the asyncssh library and writing the EOF when the writer stream is closed. It avoids to re-open the connection before that previous one was completely closed.

Finally, three logging messages were fixed. A Docker DEBUG logging message has new line at the end of the message. The step exit status was in lowercase, but in StreamFlow the status is showed in uppercase. The logging of commands executed remotely though SSHConnector were not printed because there was an wrong operation association between an inline if-else and two f-strings.

@LanderOtto LanderOtto force-pushed the fix/retry branch 12 times, most recently from d19da72 to a387d0a Compare July 18, 2024 22:17
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

Attention: Patch coverage is 77.55102% with 11 lines in your changes missing coverage. Please review.

Project coverage is 69.90%. Comparing base (d3c8b95) to head (14b696b).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
streamflow/deployment/connector/ssh.py 77.55% 6 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
- Coverage   69.94%   69.90%   -0.04%     
==========================================
  Files          87       87              
  Lines       10882    10889       +7     
  Branches     2548     2548              
==========================================
+ Hits         7611     7612       +1     
- Misses       2798     2802       +4     
- Partials      473      475       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LanderOtto LanderOtto force-pushed the fix/retry branch 2 times, most recently from cf1268f to 8a57839 Compare September 20, 2024 11:55
@LanderOtto LanderOtto force-pushed the fix/retry branch 8 times, most recently from 7b6b3e8 to 2736cb7 Compare September 25, 2024 11:53
@LanderOtto LanderOtto force-pushed the fix/retry branch 2 times, most recently from 2a28510 to 14d0cef Compare October 4, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant