Fix suite info on daemonize #2044
Merged
Conversation
This file contains 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
matthewrmshin
force-pushed
the
fix-suite-info-on-daemonize
branch
3 times, most recently
from
1b37e2b
to
fe9b4e6
Compare
matthewrmshin
force-pushed
the
fix-suite-info-on-daemonize
branch
from
fe9b4e6
to
cdfb0f9
Compare
arjclark
reviewed
Oct 19, 2016
| @@ -2060,7 +2064,6 @@ def will_pause_at(self): | |||
|
|
|||
| def command_trigger_tasks(self, items): | |||
| """Trigger tasks.""" | |||
| print "Trigger", items | |||
There was a problem hiding this comment.
Why is this being removed?
There was a problem hiding this comment.
This looks like a left behind debug print statement. The command and its arguments should already be logged by the process_command_queue method.
|
Review 2 Looks OK to me. No problems found from the test-battery in my environment. |
hjoliver
reviewed
Oct 25, 2016
| # Poll for suite log to be populated | ||
| suite_pid = None | ||
| suite_port = None | ||
| while suite_pid is None or suite_port is None: |
There was a problem hiding this comment.
Should this loop time out after a while in case something stops the log file being written?
There was a problem hiding this comment.
Very unlikely, as the log file should at least have some errors in the 1st line - which would result in the loop exiting with an error.
However, just be on the safe side, I have now added a timeout after 5 minutes.
cylc run/restart command will now poll for suite log to be populated with the start up line so it can get the information on PID and port number and print them out correctly to the user. Incidentally, this also allows the suite to exit 1 if the suite does not start up correctly due to suite configuration errors.
matthewrmshin
force-pushed
the
fix-suite-info-on-daemonize
branch
from
e643593
to
e6f1f79
Compare
|
Branch re-based. |
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.
cylc run/restart command will now poll for suite log to be populated
with the start up line so it can get the information on PID and port number and print them out correctly to the user.
Added suite PID to start up line in the suite log.
Incidentally, this also allows the suite to exit 1 if the suite does not
start up correctly due to suite configuration errors, etc.
Also migrated remaining useful logic in
cylc.suite_outputintocylc.daemonize.Fix #2041. Fix #1720.