-
Notifications
You must be signed in to change notification settings - Fork 93
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
Implemented retry for task messaging commands: #114 #188
Implemented retry for task messaging commands: #114 #188
Conversation
This addresses Issue #114 |
Should we put task messaging calls in the background so the retry process does not actually hold up the task? |
It would be a good idea to background the messaging calls. |
You'd need to ensure that any previous message had completed before trying to send a new message? |
The first cut did not keep the validation-expanded user file so type coercion of non-string user values was not retained (this is somewhat tricky because we are using a single configspec to set default values for two config files (user and site) that combine to get the result.)
(I think this is done now). |
Appears to be working as expected. However, conflicts arise on merging with current master and parts of cylc seem to break once a merge is complete relating to:
It may be that I'm failing to correctly resolve the conflicts by hand. @hjoliver could you take a look at this? |
Conflicts: bin/cylc-submit conf/siterc/cfgspec lib/cylc/scheduler.py
@arjclark I've merged from master and addressed the conflicts - try again now. Apologies for the hassle - in future I'll do this as a matter of course whenever github says the merge can't be done automatically. |
Tested as working our end. Thanks @hjoliver for taking a look at that. |
Implemented retry for task messaging commands: #114
To test this:
Note that the connection timeout does not apply if "connection failed" occurs (no suite), only if a connection is made but is not completed for some reason (suspend a suite with Ctrl-Z to see this).