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

Don't delay reload of active tasks. #1835

Merged
merged 1 commit into from May 16, 2016

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented May 12, 2016

Close #1818 - simplify reload logic by not delaying reload of active tasks. Instead just log a warning that the job is already active under the old settings.

Now preserves task outputs across the reload, so there's no need for the delayed reload any more even if task outputs are changed in the reloaded suite definition.

@hjoliver hjoliver added this to the soon milestone May 12, 2016
@hjoliver hjoliver force-pushed the 1818.reload-active-tasks-now branch 2 times, most recently from 5044375 to 61f7950 Compare May 13, 2016 02:53
@hjoliver hjoliver force-pushed the 1818.reload-active-tasks-now branch from 61f7950 to f1daaf3 Compare May 14, 2016 01:30
@hjoliver hjoliver self-assigned this May 14, 2016
@@ -821,7 +844,6 @@ def job_submission_failed(self):
msg = "submission failed, %s (after %s)" % (delay_msg, timeout_str)
self.log(INFO, "job(%02d) " % self.submit_num + msg)
self.summary['latest_message'] = msg
self.summary['waiting for reload'] = self.reconfigure_me
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used

@hjoliver hjoliver force-pushed the 1818.reload-active-tasks-now branch 2 times, most recently from 34e562f to f146028 Compare May 14, 2016 02:59
@hjoliver hjoliver force-pushed the 1818.reload-active-tasks-now branch 2 times, most recently from a5f11df to 5e02e58 Compare May 14, 2016 03:38
@hjoliver
Copy link
Member Author

Tests pass here.

@matthewrmshin - please review/assign.

@matthewrmshin
Copy link
Contributor

Looks OK. Tests OK (with and without global configuration) in my environment.

@arjclark please review.

@arjclark
Copy link
Contributor

Looks OK to me. No problems from the test-battery in my environment. @matthewrmshin informs me he's done some manual testing too to confirm change is working.

@arjclark arjclark merged commit e7928d2 into cylc:master May 16, 2016
@matthewrmshin matthewrmshin modified the milestones: next release, soon May 16, 2016
@hjoliver hjoliver deleted the 1818.reload-active-tasks-now branch May 17, 2016 08:40
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.

None yet

3 participants