Implement limits group background task #488
Conversation
ghost
requested review from
|
Let me know what you think about adding documentation for this feature. Should it go under System Configuration at the top somewhere? User Guides? Tips? |
| @@ -794,12 +794,14 @@ def get_limits_groups | |||
|
|
|||
| # (see Cosmos::Limits#enable_group) | |||
| def enable_limits_group(group_name) | |||
| Logger.info("Enabling Limits Group: #{group_name.upcase}") | |||
There was a problem hiding this comment.
What do you think about putting the log message right in the API. I noticed we did that for enable_limits_group but not for a bunch of other things.
| # so it won't satisfy any of the other checks and enable the group | ||
| self.instance_variable_set(var_name, FUTURE_TIME) | ||
| @status = "Enabling group #{group.upcase} at #{Time.now}" | ||
| #Logger.info "Enabling group #{group.upcase}" |
There was a problem hiding this comment.
Without the logging in the API I need to implement it here
| super(initial_delay) | ||
| @temp1_enable_code = Proc.new do | ||
| #Logger.info "Enabling group GROUND" | ||
| enable_limits_group('GROUND') |
There was a problem hiding this comment.
I noticed the logging when I didn't see the message for this enable_limits_group. That's why I think it should be in the API.
|
Also, yes please create a ticket for this. |
| end | ||
| end | ||
|
|
||
| def call |
There was a problem hiding this comment.
You should use the @status variable to report status from the background task
|
closes #489 |
| # won't satisfy any of the other checks and disable the group | ||
| if !var || var == FUTURE_TIME | ||
| self.instance_variable_set(var_name, PAST_TIME) | ||
| @status = "Disabling group #{group.upcase} at #{Time.now}" |
There was a problem hiding this comment.
Status should more be used for checking_limits/sleeping. Disabling group will show up in the messages.
There was a problem hiding this comment.
It only displays a single line so I thought it made sense to display the enable/disable actions
There was a problem hiding this comment.
That makes sense, but the line will quickly become stale...
|
I would like to see something that shows the task is alive on the CmdTlmServer Status tab. Maybe we need to add a run_count variable that counts every time the task runs? |
|
Fix specs and then this looks good. |
|
👍 |
Let me know if you want me to create a ticket for this. Also I'm open to comments about naming conventions.