Skip to content

Commit 4b3ccb7

Browse files
chrisburrchrisburr
committed
refactor: migrate getDIRACPlatform to DIRACCommon
Migrate getDIRACPlatform function from DIRAC to DIRACCommon to enable shared usage: - Move getDIRACPlatform and _platformSortKey to DIRACCommon/ConfigurationSystem/Client/Helpers/Resources.py - Create stateless DIRACCommon implementation that accepts configuration data as parameters - Update DIRAC module to import from DIRACCommon for backward compatibility - DIRAC wrapper handles gConfig access and passes data to DIRACCommon function - Move and update tests for both DIRACCommon and DIRAC modules - Update DIRACCommon README with comprehensive migration documentation
1 parent 2d52fb5 commit 4b3ccb7

File tree

11 files changed

+374
-56
lines changed

11 files changed

+374
-56
lines changed

‎dirac-common/README.md‎

Lines changed: 201 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,16 @@ This package solves the circular dependency issue where DiracX needs DIRAC utili
1111
- `DIRACCommon.Core.Utilities.ReturnValues`: DIRAC's S_OK/S_ERROR return value system
1212
- `DIRACCommon.Core.Utilities.DErrno`: DIRAC error codes and utilities
1313
- `DIRACCommon.Core.Utilities.ClassAd.ClassAdLight`: JDL parsing utilities
14+
- `DIRACCommon.Core.Utilities.TimeUtilities`: Time and date utilities
15+
- `DIRACCommon.Core.Utilities.StateMachine`: State machine utilities
16+
- `DIRACCommon.Core.Utilities.JDL`: JDL parsing utilities
17+
- `DIRACCommon.Core.Utilities.List`: List manipulation utilities
18+
- `DIRACCommon.ConfigurationSystem.Client.Helpers.Resources`: Platform compatibility utilities
19+
- `DIRACCommon.WorkloadManagementSystem.Client.JobStatus`: Job status constants and state machines
20+
- `DIRACCommon.WorkloadManagementSystem.Client.JobState.JobManifest`: Job manifest utilities
1421
- `DIRACCommon.WorkloadManagementSystem.DB.JobDBUtils`: Job database utilities
22+
- `DIRACCommon.WorkloadManagementSystem.Utilities.JobModel`: Pydantic-based job models
23+
- `DIRACCommon.WorkloadManagementSystem.Utilities.JobStatusUtility`: Job status utilities
1524
- `DIRACCommon.WorkloadManagementSystem.Utilities.ParametricJob`: Parametric job utilities
1625

1726
## Installation
@@ -41,10 +50,196 @@ pixi install
4150
pixi run pytest
4251
```
4352

44-
## Guidelines for Adding Code
53+
## Migrating Code to DIRACCommon
4554

46-
Code added to DIRACCommon must:
47-
- Be completely stateless
48-
- Not import or use any of DIRAC's global objects (`gConfig`, `gLogger`, `gMonitor`, `Operations`)
49-
- Not establish database connections
50-
- Not have side effects on import
55+
This section documents the proper pattern for moving code from DIRAC to DIRACCommon to enable shared usage by DiracX and other projects.
56+
57+
### Migration Pattern
58+
59+
The migration follows a specific pattern to maintain backward compatibility while making code stateless:
60+
61+
1. **Move core functionality to DIRACCommon** - Create the stateless version
62+
2. **Update DIRAC module** - Make it a backward compatibility wrapper
63+
3. **Move and update tests** - Ensure both versions are tested
64+
4. **Verify migration** - Test both import paths work correctly
65+
66+
### Step-by-Step Migration Process
67+
68+
#### 1. Create DIRACCommon Module
69+
70+
Create the new module in DIRACCommon with the **exact same directory structure** as DIRAC:
71+
72+
```bash
73+
# Example: Moving from src/DIRAC/ConfigurationSystem/Client/Helpers/Resources.py
74+
# Create: dirac-common/src/DIRACCommon/ConfigurationSystem/Client/Helpers/Resources.py
75+
```
76+
77+
#### 2. Make Code Stateless
78+
79+
**❌ Remove these dependencies:**
80+
```python
81+
# DON'T import these in DIRACCommon
82+
from DIRAC import gConfig, gLogger, gMonitor, Operations
83+
from DIRAC.Core.Security import getProxyInfo
84+
# Any other DIRAC global state
85+
```
86+
87+
**✅ Use these instead:**
88+
```python
89+
# Use DIRACCommon's own utilities
90+
from DIRACCommon.Core.Utilities.ReturnValues import S_OK, S_ERROR
91+
from DIRACCommon.Core.Utilities.DErrno import strerror
92+
93+
# Accept configuration data as parameters
94+
def my_function(data, config_dict):
95+
# Use config_dict instead of gConfig.getOptionsDict()
96+
pass
97+
```
98+
99+
#### 3. Handle Configuration Data
100+
101+
**❌ Don't do this:**
102+
```python
103+
# DIRACCommon function taking config object
104+
def getDIRACPlatform(OSList, config):
105+
result = config.getOptionsDict("/Resources/Computing/OSCompatibility")
106+
# ...
107+
```
108+
109+
**✅ Do this instead:**
110+
```python
111+
# DIRACCommon function taking configuration data directly
112+
def getDIRACPlatform(OSList, osCompatibilityDict):
113+
if not osCompatibilityDict:
114+
return S_ERROR("OS compatibility info not found")
115+
# Use osCompatibilityDict directly
116+
# ...
117+
```
118+
119+
#### 4. Update DIRAC Module for Backward Compatibility
120+
121+
Transform the original DIRAC module into a backward compatibility wrapper:
122+
123+
```python
124+
"""Backward compatibility wrapper - moved to DIRACCommon
125+
126+
This module has been moved to DIRACCommon.ConfigurationSystem.Client.Helpers.Resources
127+
to avoid circular dependencies and allow DiracX to use these utilities without
128+
triggering DIRAC's global state initialization.
129+
130+
All exports are maintained for backward compatibility.
131+
"""
132+
133+
# Re-export everything from DIRACCommon for backward compatibility
134+
from DIRACCommon.ConfigurationSystem.Client.Helpers.Resources import (
135+
getDIRACPlatform as _getDIRACPlatform,
136+
_platformSortKey,
137+
)
138+
139+
from DIRAC import S_ERROR, S_OK, gConfig
140+
141+
def getDIRACPlatform(OSList):
142+
"""Get standard DIRAC platform(s) compatible with the argument.
143+
144+
Backward compatibility wrapper that uses gConfig.
145+
"""
146+
result = gConfig.getOptionsDict("/Resources/Computing/OSCompatibility")
147+
if not (result["OK"] and result["Value"]):
148+
return S_ERROR("OS compatibility info not found")
149+
150+
return _getDIRACPlatform(OSList, result["Value"])
151+
152+
# Re-export the helper function
153+
_platformSortKey = _platformSortKey
154+
```
155+
156+
#### 5. Move and Update Tests
157+
158+
**Create DIRACCommon tests:**
159+
```python
160+
# dirac-common/tests/ConfigurationSystem/Client/Helpers/test_Resources.py
161+
from DIRACCommon.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform
162+
163+
def test_getDIRACPlatform():
164+
# Test with configuration data directly
165+
osCompatibilityDict = {"plat1": "OS1, OS2", "plat2": "OS3, OS4"}
166+
result = getDIRACPlatform("OS1", osCompatibilityDict)
167+
assert result["OK"]
168+
```
169+
170+
**Update DIRAC tests:**
171+
```python
172+
# src/DIRAC/ConfigurationSystem/Client/Helpers/test/Test_Helpers.py
173+
from DIRAC.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform
174+
175+
def test_getDIRACPlatform():
176+
# Test backward compatibility wrapper
177+
# (existing tests should continue to work)
178+
pass
179+
```
180+
181+
#### 6. Create Directory Structure
182+
183+
Ensure the DIRACCommon directory structure mirrors DIRAC exactly:
184+
185+
```bash
186+
dirac-common/src/DIRACCommon/
187+
├── ConfigurationSystem/
188+
│ ├── __init__.py
189+
│ └── Client/
190+
│ ├── __init__.py
191+
│ └── Helpers/
192+
│ ├── __init__.py
193+
│ └── Resources.py
194+
└── tests/
195+
└── ConfigurationSystem/
196+
├── __init__.py
197+
└── Client/
198+
├── __init__.py
199+
└── Helpers/
200+
├── __init__.py
201+
└── test_Resources.py
202+
```
203+
204+
### Requirements for DIRACCommon Code
205+
206+
Code in DIRACCommon **MUST** be:
207+
208+
- **Completely stateless** - No global variables or state
209+
- **No DIRAC dependencies** - Cannot import from DIRAC
210+
- **No global state access** - Cannot use `gConfig`, `gLogger`, `gMonitor`, etc.
211+
- **No database connections** - Cannot establish DB connections
212+
- **No side effects on import** - Importing should not trigger any initialization
213+
- **Pure functions** - Functions should be deterministic and side-effect free
214+
215+
### Configuration Data Handling
216+
217+
When DIRACCommon functions need configuration data:
218+
219+
1. **Accept data as parameters** - Don't accept config objects
220+
2. **Use specific data types** - Pass dictionaries, not config objects
221+
3. **Let DIRAC wrapper handle gConfig** - DIRAC gets data and passes it to DIRACCommon
222+
223+
### Example Migration
224+
225+
See the migration of `getDIRACPlatform` in:
226+
- `dirac-common/src/DIRACCommon/ConfigurationSystem/Client/Helpers/Resources.py`
227+
- `src/DIRAC/ConfigurationSystem/Client/Helpers/Resources.py`
228+
229+
This demonstrates the complete pattern from stateless DIRACCommon implementation to backward-compatible DIRAC wrapper.
230+
231+
### Testing the Migration
232+
233+
After migration, verify:
234+
235+
1. **DIRACCommon tests pass** - `pixi run python -m pytest dirac-common/tests/`
236+
2. **DIRAC tests pass** - `pixi run python -m pytest src/DIRAC/`
237+
3. **Both import paths work**:
238+
```python
239+
# DIRACCommon (stateless)
240+
from DIRACCommon.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform
241+
242+
# DIRAC (backward compatibility)
243+
from DIRAC.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform
244+
```
245+
4. **No linting errors** - All code should pass linting checks

‎dirac-common/src/DIRACCommon/ConfigurationSystem/Client/Helpers/Resources.py‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Platform utilities for DIRAC platform compatibility and management.
2+
3+
This module provides functions for working with DIRAC platforms and OS compatibility.
4+
"""
5+
import re
6+
7+
from DIRACCommon.Core.Utilities.ReturnValues import S_ERROR, S_OK
8+
9+
10+
def getDIRACPlatform(OSList, osCompatibilityDict):
11+
"""Get standard DIRAC platform(s) compatible with the argument.
12+
13+
NB: The returned value is a list, ordered by numeric components in the platform.
14+
In practice the "highest" version (which should be the most "desirable" one is returned first)
15+
16+
:param list OSList: list of platforms defined by resource providers
17+
:param dict osCompatibilityDict: dictionary with OS compatibility information
18+
:return: a list of DIRAC platforms that can be specified in job descriptions
19+
"""
20+
21+
# For backward compatibility allow a single string argument
22+
osList = OSList
23+
if isinstance(OSList, str):
24+
osList = [OSList]
25+
26+
if not osCompatibilityDict:
27+
return S_ERROR("OS compatibility info not found")
28+
29+
platformsDict = {k: v.replace(" ", "").split(",") for k, v in osCompatibilityDict.items()} # can be an iterator
30+
for k, v in platformsDict.items(): # can be an iterator
31+
if k not in v:
32+
v.append(k)
33+
34+
# making an OS -> platforms dict
35+
os2PlatformDict = dict()
36+
for platform, osItems in platformsDict.items(): # can be an iterator
37+
for osItem in osItems:
38+
if os2PlatformDict.get(osItem):
39+
os2PlatformDict[osItem].append(platform)
40+
else:
41+
os2PlatformDict[osItem] = [platform]
42+
43+
platforms = []
44+
for os in osList:
45+
if os in os2PlatformDict:
46+
platforms += os2PlatformDict[os]
47+
48+
if not platforms:
49+
return S_ERROR(f"No compatible DIRAC platform found for {','.join(OSList)}")
50+
51+
platforms.sort(key=_platformSortKey, reverse=True)
52+
53+
return S_OK(platforms)
54+
55+
56+
def _platformSortKey(version: str) -> list[str]:
57+
# Loosely based on distutils.version.LooseVersion
58+
parts = []
59+
for part in re.split(r"(\d+|[a-z]+|\.| -)", version.lower()):
60+
if not part or part == ".":
61+
continue
62+
if part[:1] in "0123456789":
63+
part = part.zfill(8)
64+
else:
65+
while parts and parts[-1] == "00000000":
66+
parts.pop()
67+
parts.append(part)
68+
return parts

‎dirac-common/src/DIRACCommon/ConfigurationSystem/Client/Helpers/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem.Client.Helpers - Configuration system helper functions
3+
"""

‎dirac-common/src/DIRACCommon/ConfigurationSystem/Client/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem.Client - Configuration system client components
3+
"""

‎dirac-common/src/DIRACCommon/ConfigurationSystem/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem - Configuration system components
3+
"""

‎dirac-common/tests/ConfigurationSystem/Client/Helpers/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem.Client.Helpers tests
3+
"""

‎dirac-common/tests/ConfigurationSystem/Client/Helpers/test_Resources.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
from itertools import zip_longest
2+
3+
import pytest
4+
5+
from DIRACCommon.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform, _platformSortKey
6+
7+
8+
@pytest.mark.parametrize(
9+
"mockGCReplyInput, requested, expectedRes, expectedValue",
10+
[
11+
({"OK": False, "Message": "error"}, "plat", False, None),
12+
({"OK": True, "Value": ""}, "plat", False, None),
13+
(
14+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
15+
"plat",
16+
False,
17+
None,
18+
),
19+
(
20+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
21+
"OS1",
22+
True,
23+
["plat1", "plat3"],
24+
),
25+
(
26+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
27+
"OS2",
28+
True,
29+
["plat1"],
30+
),
31+
(
32+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
33+
"OS3",
34+
True,
35+
["plat1"],
36+
),
37+
(
38+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
39+
"OS4",
40+
True,
41+
["plat2", "plat3"],
42+
),
43+
(
44+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
45+
"OS5",
46+
True,
47+
["plat2"],
48+
),
49+
(
50+
{"OK": True, "Value": {"plat1": "OS1, OS2, OS3", "plat2": "OS4, OS5", "plat3": "OS1, OS4"}},
51+
"plat1",
52+
True,
53+
["plat1"],
54+
),
55+
],
56+
)
57+
def test_getDIRACPlatform(mockGCReplyInput, requested, expectedRes, expectedValue):
58+
# Extract the configuration data from the mock input
59+
osCompatibilityDict = mockGCReplyInput.get("Value", {}) if mockGCReplyInput.get("OK") else {}
60+
61+
res = getDIRACPlatform(requested, osCompatibilityDict)
62+
assert res["OK"] is expectedRes, res
63+
if expectedRes:
64+
assert set(res["Value"]) == set(expectedValue), res["Value"]
65+
66+
67+
@pytest.mark.parametrize(
68+
"string,expected",
69+
[
70+
("Darwin_arm64_12.4", ["darwin", "_", "arm", "64", "_", "12", "4"]),
71+
("Linux_x86_64_glibc-2.17", ["linux", "_", "x", "86", "_", "64", "_", "glibc", "-", "2", "17"]),
72+
("Linux_aarch64_glibc-2.28", ["linux", "_", "aarch", "64", "_", "glibc", "-", "2", "28"]),
73+
],
74+
)
75+
def test_platformSortKey(string, expected):
76+
actual = _platformSortKey(string)
77+
for a, e in zip_longest(actual, expected):
78+
# Numbers are padded with zeros so string comparison works
79+
assert a.lstrip("0") == e

‎dirac-common/tests/ConfigurationSystem/Client/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem.Client tests
3+
"""

‎dirac-common/tests/ConfigurationSystem/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
DIRACCommon.ConfigurationSystem tests
3+
"""

0 commit comments

Comments
 (0)