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

Update chronos_serviceinit to use LastRunState #5

Merged
merged 4 commits into from
Oct 29, 2015

Conversation

Rob-Johnson
Copy link
Contributor

The logic performed to calculate the formatted string in chronos_serviceinit is now duplicating that available in chronos_tools.get_status_last_run for calculating LastRunState. This removes that duplication, as well as updating chronos_tools.get_status_last_run to return the date string with the state.

I've updated tests + done some manual testing.

Here's a screenshot of the output:

screen shot 2015-10-28 at 15 13 49

@Rob-Johnson Rob-Johnson force-pushed the chronos-serviceinit-use-lastrunstate branch from cc8118b to 5bbeffe Compare October 28, 2015 15:34
@@ -61,7 +61,7 @@ def send_event_to_sensu(service, instance, monitoring_overrides, soa_dir, status

def last_run_state_for_jobs(jobs):
"""Map over a list of jobs to create a pair of (job, LasRunState)"""
return [(chronos_job, chronos_tools.get_status_last_run(chronos_job)) for chronos_job in jobs]
return [(chronos_job, chronos_tools.get_status_last_run(chronos_job)[-1]) for chronos_job in jobs]
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the negative one in this case, is it the most recent run? If so can you make a comment explaining that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it's the lastrunstate in the pair of (time, lastrunstate) that's returned by chronos_tools.get_status_last_run(chronos_job). I'll add a comment.

@solarkennedy
Copy link
Contributor

fix + ship

@@ -202,7 +197,7 @@ def format_chronos_job_status(job, desired_state, running_tasks, verbose):
"""
config_hash = _format_config_hash(job)
disabled_state = _format_disabled_status(job)
(last_result, last_result_when) = _format_last_result(job)
last_result, formatted_time = _format_last_result(job)
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep these parens.

@mrtyler
Copy link
Contributor

mrtyler commented Oct 29, 2015

fix'n'ship

@Rob-Johnson Rob-Johnson merged commit 0dfb0c3 into master Oct 29, 2015
@Rob-Johnson Rob-Johnson deleted the chronos-serviceinit-use-lastrunstate branch November 10, 2015 17:39
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.

3 participants