Improve the human readable state of services depending on their type #133
Conversation
| @@ -394,6 +395,15 @@ def get_healthcheck_mode(self, _): | |||
| # Healthchecks are not supported yet in chronos | |||
| return None | |||
|
|
|||
| def get_desired_state_human(self): | |||
There was a problem hiding this comment.
I don't think this should be an instance level function, just a module function that takes a state and returns a human readable version (so the get_desired_state is moved to the caller). Same with the version in marathon tools. I know this change is like for like as is, but still, worth cleaning up if you're touching the code. To do this you'll need to edit chronos_serviceinit + marathon_serviceinit to change the caller to call the right function.
There was a problem hiding this comment.
And then should the caller inject what type of instance it needs the output for? Should it inject the whole object? Otherwise the caller would have to "know" to inject instances or whatever.
There was a problem hiding this comment.
if you have the marathon-version of get_desired_state_human in marathon_tools and a chronos-version in chronos_tools, then the caller will have to make a decision on which to call. This is fine for now because the caller is in marathon_serviceinit and chronos_serviceinit, so you'll have to update each of those to call chronos_tools.get_desired_state_human and marathon_tools.get_desired_state_human respectively.
There was a problem hiding this comment.
This is not actually true. These are class functions that for the serviceinstanceconfig.
A caller doesn't need to "know" anything. (notice that this patch required no changes to the client code, they were calling get_desired_state_human just as before) So they just run service_config.get_desired_state_human like normal.
All I'm doing is taking a class function that was too generic in serviceinstanceconfig and adding framework-specific versions that are more specific. (and hopefully more human readable of course)
|
fix n ship |
Improve the human readable state of services depending on their type
This was a feature request from @gchin.
In the beginning when we only had marathon, this logic made sense. Now I think it can be better if it the human readable output was more "aware" of what kind of job it is, which influences what it means to be "stopped or started".
At the same time, a service with 0 instances is considered "stopped" when you run paasta stop, so we we improve this message and just say "stopped", even if you didn't literally use the paasta stop command?