Skip to content

Commit 4b9de24

Browse files
authored
Merge pull request #8295 from chrisburr/common-getDIRACPlatform
[9.0] migrate getDIRACPlatform to DIRACCommon
2 parents 931e647 + bd09645 commit 4b9de24

File tree

11 files changed

+341
-65
lines changed

11 files changed

+341
-65
lines changed

dirac-common/README.md

Lines changed: 213 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
@@ -22,6 +31,8 @@ pip install DIRACCommon
2231

2332
## Usage
2433

34+
### Basic Usage
35+
2536
```python
2637
from DIRACCommon.Core.Utilities.ReturnValues import S_OK, S_ERROR
2738

@@ -41,10 +52,206 @@ pixi install
4152
pixi run pytest
4253
```
4354

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

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
254+
# DIRAC (backward compatibility)
255+
from DIRAC.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform
256+
```
257+
4. **No linting errors** - All code should pass linting checks
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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, DReturnType
8+
9+
10+
def getDIRACPlatform(osList: list[str], osCompatibilityDict: dict[str, set[str]]) -> DReturnType[list[str]]:
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+
if not osCompatibilityDict:
22+
return S_ERROR("OS compatibility info not found")
23+
24+
# making an OS -> platforms dict
25+
os2PlatformDict = dict()
26+
for platform, osItems in osCompatibilityDict.items():
27+
for osItem in osItems:
28+
os2PlatformDict.setdefault(osItem, set()).add(platform)
29+
30+
platforms = set()
31+
for os in osList:
32+
platforms |= os2PlatformDict.get(os, set())
33+
34+
if not platforms:
35+
return S_ERROR(f"No compatible DIRAC platform found for {','.join(osList)}")
36+
37+
return S_OK(sorted(platforms, key=_platformSortKey, reverse=True))
38+
39+
40+
def _platformSortKey(version: str) -> list[str]:
41+
# Loosely based on distutils.version.LooseVersion
42+
parts = []
43+
for part in re.split(r"(\d+|[a-z]+|\.| -)", version.lower()):
44+
if not part or part == ".":
45+
continue
46+
if part[:1] in "0123456789":
47+
part = part.zfill(8)
48+
else:
49+
while parts and parts[-1] == "00000000":
50+
parts.pop()
51+
parts.append(part)
52+
return parts
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+
"""
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+
"""
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+
"""
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+
"""
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
from itertools import zip_longest
2+
3+
import pytest
4+
5+
from DIRACCommon.ConfigurationSystem.Client.Helpers.Resources import getDIRACPlatform, _platformSortKey
6+
7+
8+
MOCK_OS_COMPATIBILITY_DICT = {
9+
"plat1": {"plat1", "OS1", "OS2", "OS3"},
10+
"plat2": {"plat2", "OS4", "OS5"},
11+
"plat3": {"plat3", "OS1", "OS4"},
12+
}
13+
14+
15+
@pytest.mark.parametrize(
16+
"osCompatibilityDict, osList, expectedRes, expectedValue",
17+
[
18+
({}, ["plat"], False, None),
19+
(MOCK_OS_COMPATIBILITY_DICT, ["plat"], False, None),
20+
(MOCK_OS_COMPATIBILITY_DICT, ["OS1"], True, ["plat1", "plat3"]),
21+
(MOCK_OS_COMPATIBILITY_DICT, ["OS2"], True, ["plat1"]),
22+
(MOCK_OS_COMPATIBILITY_DICT, ["OS3"], True, ["plat1"]),
23+
(MOCK_OS_COMPATIBILITY_DICT, ["OS4"], True, ["plat2", "plat3"]),
24+
(MOCK_OS_COMPATIBILITY_DICT, ["OS5"], True, ["plat2"]),
25+
(MOCK_OS_COMPATIBILITY_DICT, ["plat1"], True, ["plat1"]),
26+
],
27+
)
28+
def test_getDIRACPlatform(osCompatibilityDict, osList, expectedRes, expectedValue):
29+
res = getDIRACPlatform(osList, osCompatibilityDict)
30+
assert res["OK"] is expectedRes, res
31+
if expectedRes:
32+
assert set(res["Value"]) == set(expectedValue), res["Value"]
33+
34+
35+
@pytest.mark.parametrize(
36+
"string,expected",
37+
[
38+
("Darwin_arm64_12.4", ["darwin", "_", "arm", "64", "_", "12", "4"]),
39+
("Linux_x86_64_glibc-2.17", ["linux", "_", "x", "86", "_", "64", "_", "glibc", "-", "2", "17"]),
40+
("Linux_aarch64_glibc-2.28", ["linux", "_", "aarch", "64", "_", "glibc", "-", "2", "28"]),
41+
],
42+
)
43+
def test_platformSortKey(string, expected):
44+
actual = _platformSortKey(string)
45+
for a, e in zip_longest(actual, expected):
46+
# Numbers are padded with zeros so string comparison works
47+
assert a.lstrip("0") == e
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+
"""
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)