Skip to content

Commit 603dbf3

Browse files
committed
refactor: commands read args directly from self.args without hardcoded defaults
FreeDiskSpaceCommand and DowntimeCommand no longer duplicate default values in .get() fallbacks — defaults live exclusively in POLICIESMETA and are guaranteed to be present in self.args by the time the command runs.
1 parent b6b0340 commit 603dbf3

7 files changed

Lines changed: 210 additions & 192 deletions

File tree

docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ Cache tables for metrics used by policies.
115115
* - PolicyResult
116116
- Policy evaluation results (Element, Name, PolicyName, Status, Reason)
117117
* - SpaceTokenOccupancyCache
118-
- Storage space usage (Endpoint, Token, Free, Total) — values stored in MB
118+
- Storage space usage (Endpoint, Token, Free, Total) — values stored in MB
119119
* - TransferCache
120120
- Transfer quality metrics (SourceName, DestinationName, Metric, Value)
121121

src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
Downtimes found are stored in the DowntimeCache table via ResourceManagementClient.
55
Stale or deleted GOCDB downtimes are also removed from the cache.
66
7-
The look-ahead window is controlled by the ``hours`` argument (read from ``self.args``,
8-
populated from ``/Operations/Defaults/ResourceStatus/Policies/Downtime/hours``):
7+
The look-ahead window is controlled by the ``hours`` argument read from ``self.args``,
8+
populated by the policy engine from ``POLICIESMETA`` defaults and any CS overrides:
99
10-
* ``hours = 0`` (default) — only ongoing downtimes are considered.
11-
* ``hours > 0`` — downtimes starting within the next ``hours`` hours are also included.
10+
* ``hours = 0`` — only ongoing downtimes are considered.
11+
* ``hours > 0`` — downtimes starting within the next ``hours`` hours are also included.
1212
1313
"""
1414

@@ -144,7 +144,7 @@ def _prepareCommand(self):
144144
145145
Optional key:
146146
147-
* ``hours`` (int) — look-ahead window in hours (default: ``None`` = ongoing only).
147+
* ``hours`` (int) — look-ahead window in hours (populated from ``POLICIESMETA``).
148148
149149
Name resolution:
150150
@@ -172,9 +172,7 @@ def _prepareCommand(self):
172172
if element not in ["Site", "Resource"]:
173173
return S_ERROR("element is neither Site nor Resource")
174174

175-
hours = None
176-
if "hours" in self.args:
177-
hours = self.args["hours"]
175+
hours = self.args.get("hours")
178176

179177
gOCDBServiceType = None
180178

src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
33
Command to retrieve and cache the free/total disk space of a Storage Element.
44
5-
The unit and decision thresholds are read from ``self.args`` (populated from the
6-
Operations CS ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``):
5+
The unit and decision thresholds are read from ``self.args``, which are populated
6+
by the policy engine from ``POLICIESMETA`` defaults and any CS overrides:
77
8-
* ``unit`` — space unit for the occupancy query: ``TB`` (default), ``GB`` or ``MB``
9-
* ``Banned_threshold`` — free-space value below which the SE is Banned (default: 0.1)
10-
* ``Degraded_threshold`` — free-space value below which the SE is Degraded (default: 5)
8+
* ``unit`` — space unit for the occupancy query (``TB``, ``GB`` or ``MB``)
9+
* ``Banned_threshold`` — free-space value below which the SE is Banned
10+
* ``Degraded_threshold`` — free-space value below which the SE is Degraded
1111
1212
Note: there are still many references to "space tokens" (e.g.
1313
``ResourceManagementClient().selectSpaceTokenOccupancyCache(token=elementName)``).
@@ -52,11 +52,11 @@ def _prepareCommand(self):
5252
5353
* ``name`` (str) — Storage Element name.
5454
55-
Optional keys (all read from the FreeDiskSpace policy configuration):
55+
Optional keys (populated from ``POLICIESMETA`` defaults and CS overrides):
5656
57-
* ``unit`` (str) — space unit: ``TB`` (default), ``GB`` or ``MB``.
58-
* ``Banned_threshold`` (float) — free space below which the SE is Banned (default: 0.1).
59-
* ``Degraded_threshold`` (float) — free space below which the SE is Degraded (default: 5).
57+
* ``unit`` (str) — space unit: ``TB``, ``GB`` or ``MB``.
58+
* ``Banned_threshold`` (float) — free space below which the SE is Banned.
59+
* ``Degraded_threshold`` (float) — free space below which the SE is Degraded.
6060
6161
:returns: S_OK tuple ``(elementName, unit, banned_threshold, degraded_threshold)``
6262
or S_ERROR if ``name`` is missing.
@@ -66,10 +66,9 @@ def _prepareCommand(self):
6666
return S_ERROR('"name" not found in self.args')
6767
elementName = self.args["name"]
6868

69-
unit = self.args.get("unit", "TB")
70-
71-
banned_threshold = self.args.get("Banned_threshold", 0.1)
72-
degraded_threshold = self.args.get("Degraded_threshold", 5)
69+
unit = self.args["unit"]
70+
banned_threshold = self.args["Banned_threshold"]
71+
degraded_threshold = self.args["Degraded_threshold"]
7372

7473
return S_OK((elementName, unit, banned_threshold, degraded_threshold))
7574

@@ -91,8 +90,8 @@ def doNew(self, masterParams=None):
9190

9291
if masterParams is not None:
9392
elementName, unit = masterParams
94-
banned_threshold = self.args.get("Banned_threshold", 0.1)
95-
degraded_threshold = self.args.get("Degraded_threshold", 5)
93+
banned_threshold = self.args["Banned_threshold"]
94+
degraded_threshold = self.args["Degraded_threshold"]
9695
else:
9796
params = self._prepareCommand()
9897
if not params["OK"]:

src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def _evaluate(commandResult):
2828
2929
:param dict commandResult: S_OK / S_ERROR result from FreeDiskSpaceCommand.
3030
On success the value is expected to be a dict with keys:
31-
``Free``, ``Total``, ``Banned_threshold`` (optional), ``Degraded_threshold`` (optional).
31+
``Free``, ``Total``, ``Banned_threshold``, ``Degraded_threshold``.
3232
3333
:returns: S_OK wrapping a dict ``{'Status': str, 'Reason': str}`` where Status is one of
3434
``Error``, ``Unknown``, ``Banned``, ``Degraded``, ``Active``.
@@ -58,10 +58,10 @@ def _evaluate(commandResult):
5858

5959
# Units (TB, GB, MB) may change,
6060
# depending on the configuration of the command in Configurations.py
61-
if free < commandResult.get("Banned_threshold", 0.1):
61+
if free < commandResult["Banned_threshold"]: # default: 0.1
6262
result["Status"] = "Banned"
6363
result["Reason"] = "Too little free space"
64-
elif free < commandResult.get("Degraded_threshold", 5):
64+
elif free < commandResult["Degraded_threshold"]: # default: 5
6565
result["Status"] = "Degraded"
6666
result["Reason"] = "Little free space"
6767
else:

src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
""" Test_RSS_Policy_FreeDiskSpacePolicy
2-
"""
1+
"""Test_RSS_Policy_FreeDiskSpacePolicy"""
32
# pylint: disable=protected-access
43

54
import unittest
@@ -72,17 +71,29 @@ def test_evaluate(self):
7271
self.assertEqual("Error", res["Value"]["Status"])
7372
self.assertEqual("Key Free missing", res["Value"]["Reason"])
7473

75-
res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 0.0}})
74+
res = module._evaluate(
75+
{"OK": True, "Value": {"Total": 100, "Free": 0.0, "Banned_threshold": 0.1, "Degraded_threshold": 5}}
76+
)
7677
self.assertTrue(res["OK"])
7778
self.assertEqual("Banned", res["Value"]["Status"])
7879
self.assertEqual("Too little free space", res["Value"]["Reason"])
7980

80-
res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 4.0, "Guaranteed": 1}})
81+
res = module._evaluate(
82+
{
83+
"OK": True,
84+
"Value": {"Total": 100, "Free": 4.0, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5},
85+
}
86+
)
8187
self.assertTrue(res["OK"])
8288
self.assertEqual("Degraded", res["Value"]["Status"])
8389
self.assertEqual("Little free space", res["Value"]["Reason"])
8490

85-
res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 100, "Guaranteed": 1}})
91+
res = module._evaluate(
92+
{
93+
"OK": True,
94+
"Value": {"Total": 100, "Free": 100, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5},
95+
}
96+
)
8697
self.assertTrue(res["OK"])
8798
self.assertEqual("Active", res["Value"]["Status"])
8899
self.assertEqual("Enough free space", res["Value"]["Reason"])

src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,6 @@ def postProcessingPolicyList(policiesThatApply):
285285
element, we keep only the most specific one. Specificity is determined by the number
286286
of ``matchParams`` keys: more keys = more specific. If one of the duplicates matched
287287
by ``name`` it is always considered more specific than one that did not.
288-
289-
This replaces the old per-type hacks (``FreeDiskSpaceMB`` > ``FreeDiskSpaceGB`` >
290-
``FreeDiskSpaceTB``) with a generic rule that works for any policy type.
291288
"""
292289
from collections import defaultdict
293290

0 commit comments

Comments
 (0)