disable_limits_group requiring multiple runs to disable all limits #3932

Activity

1. sunws4me

   added the

   Bug

   issue type on Sep 24, 2026
2. sunws4me

   added

   triageMore information is neededMore information is needed

   on Sep 24, 2026
3. crawlchat commented on Sep 24, 2026

   

   crawlchatbot

   on Sep 24, 2026 – with CrawlChat

   More actions

   Analysis

   Looking at the disable_limits_group implementation , I see a likely root cause for the "have to disable multiple times" symptom.

   Root cause hypothesis: exception mid-loop leaves later packets untouched

   def _limits_group(group_name, action:, manual:, scope:, token:)
     ...
     group.sort.each do |target_name, packet_name, item_name|
       if last_target_name != target_name || last_packet_name != packet_name
         if last_target_name && last_packet_name
           TargetModel.set_packet(last_target_name, last_packet_name, packet, scope: scope)
         end
         packet = TargetModel.packet(target_name, packet_name, scope: scope)
       end
       packet['items'].each do |item|
         if item['name'] == item_name
           if action == :enable
             item['limits']['enabled'] = true
           elsif action == :disable
             item['limits']['enabled'] = false   # <-- no ||= {} guard
           end
           ...
         end
       end
       last_target_name = target_name
       last_packet_name = packet_name
     end
     if last_target_name && last_packet_name
       TargetModel.set_packet(last_target_name, last_packet_name, packet, scope: scope)
     end
   end

   Notice item['limits']['enabled'] = false assumes item['limits'] already exists as a Hash. Compare this to set_state_color elsewhere in the same file, which explicitly does item['limits'] ||= {} before setting a key on it.

   If any item added via LIMITS_GROUP_ITEM does not already have a LIMITS definition on it (i.e. item['limits'] is nil), this line raises NoMethodError: undefined method '[]=' for nil:NilClass. Because packets are processed and written back to TargetModel.set_packet only when the target/packet changes (i.e., after the fact), an exception partway through a 70k‑item, multi‑packet group would:

   • Persist changes for packets already completed before the failure,
   • Abort processing for everything after the failure point (that batch's set_packet write never happens),
   • Leave the rest of the group's items still "enabled," so you'd keep seeing OOL alarms.

   Since the item order comes from group.sort, re-running disable_limits_group would make it further through the list each time before hitting the next offending item — explaining why it takes ~4 runs to fully "stick."

   What would help confirm this

   Could you check the Script Runner / cmd-tlm-api logs right after you call disable_limits_group on the 70k-item group? Look for a Ruby exception like undefined method '[]=' for nil:NilClass around the time of the call. If present, that confirms the bug is items in your LIMITS_GROUP lacking an actual LIMITS definition.

   Possible quick fix

   Adding item['limits'] ||= {} before line 428/432 (mirroring the set_state_color pattern) would prevent the exception and let the whole group process in one pass.

   If you can share a snippet of the logs from a failing disable call, that will help confirm/refute this theory before an engineer picks it up.