Skip to content

Commit 25ae4ce

Browse files
szaherclaude
andauthored
RHOAIENG-98954: type the Codeflare facade's keyword arguments (#1179)
* RHOAIENG-98954: type the Codeflare facade's keyword arguments The facade's primary entrypoints hid their arguments behind **kwargs, so a misspelled cluster field or a job with no execution target was only ever a runtime failure. cf.jobs.submit(name=..., entrypoint=...) shipping broken in - cf.clusters.create() takes **kwargs: Unpack[ClusterConfigKwargs], a TypedDict mirroring every ClusterConfiguration field except name and namespace, which the handler owns. - cf.clusters.get() drops **kwargs for explicit verify_tls / write_to_file, the only arguments get_cluster accepts. - cf.jobs.create() and submit() are overloaded on the execution target, making "exactly one of cluster_name / cluster_config" a type error rather than only a ValueError. Both now delegate to JobHandler._build(), since an overloaded method cannot satisfy its own overloads without re-narrowing the target. - ClusterConfigKwargs and JobOptions are exported, and public-surface.json gains the codeflare group it was missing for Codeflare and SDKConfig. Runtime behaviour is unchanged: the overloads are type-only and the job handler signatures are identical to before. The TypedDicts restate names that live elsewhere, so TestFacadeTypeSignatures guards them — a new ClusterConfiguration field, a changed type, a new job keyword or a dropped overload each fail a test rather than silently going unreachable through the facade. Signed-off-by: Saad Zaher <szaher@redhat.com> * RHOAIENG-98954: type the Codeflare facade's arguments The facade hid its arguments behind **kwargs, so a misspelled cluster field or a job with no execution target was only ever a runtime failure. cf.jobs.submit(name=..., entrypoint=...) shipping broken in #1091 is the case in point. - cf.clusters.create() takes a ClusterConfiguration, the same object cf.jobs.create(cluster_config=...) and Cluster() already take. One way to describe a cluster across the SDK, and the dataclass does the type checking without the facade mirroring 34 fields. - cf.clusters.get() drops **kwargs for explicit verify_tls / write_to_file, the only arguments get_cluster accepts. - cf.jobs.create() and submit() are overloaded on the execution target, making "exactly one of cluster_name / cluster_config" a type error rather than only a ValueError. Both delegate to JobHandler._build(), since an overloaded method cannot satisfy its own overloads without re-narrowing the target. - JobOptions is exported, and public-surface.json gains the codeflare group it was missing for Codeflare and SDKConfig. The handler resolves the namespace and injects it with copy.copy rather than dataclasses.replace: ClusterConfiguration.__post_init__ is not idempotent — it merges the default accelerator mapping into extended_resource_mapping and then rejects the merged result — so reconstructing a configured instance raises. Covered by test_create_accepts_a_fully_configured_config. The facade is unreleased (#1091 is in no tag), so this changes no published API. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Signed-off-by: Saad Zaher <szaher@redhat.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 9c9bf43 commit 25ae4ce

8 files changed

Lines changed: 469 additions & 99 deletions

File tree

‎AGENTS.md‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,16 @@ objects bound to that client. The lower-level `Cluster` / `RayJob` classes
1010
remain public and usable directly.
1111

1212
```python
13-
from codeflare_sdk import Codeflare, SDKConfig
13+
from codeflare_sdk import ClusterConfiguration, Codeflare, SDKConfig
1414

1515
cf = Codeflare(config=SDKConfig(namespace="my-project"))
16-
cluster = cf.clusters.create(name="my-cluster", num_workers=2)
16+
cluster = cf.clusters.create(ClusterConfiguration(name="my-cluster", num_workers=2))
1717
cluster.apply()
1818
```
1919

20+
`ClusterConfiguration` is the one way to describe a cluster: `cf.clusters.create()`,
21+
`cf.jobs.create(cluster_config=...)` and `Cluster()` all take it.
22+
2023
## Repository Structure
2124

2225
| Directory | Description |
@@ -166,6 +169,9 @@ still lists `Authentication`, `KubeConfiguration`, `TokenAuthentication` and
166169
`KubeConfigFileAuthentication`, which were removed and now raise `ImportError`
167170
(tracked in RHOAIENG-98754, pending a decision on restoring compatibility shims).
168171

172+
The `codeflare` entry covers the single entrypoint: `Codeflare`, `SDKConfig`,
173+
and the `JobOptions` TypedDict shared by the `cf.jobs` overloads.
174+
169175
Design-level architecture: `docs/designs/CodeFlare-SDK-design-doc.md`.
170176
User-facing Sphinx docs: `docs/sphinx/`.
171177

@@ -271,6 +277,14 @@ Real examples for the most common change types. Follow these patterns, not descr
271277
(`build_ray_cluster_spec`, line 96)
272278

273279
A field added to only one builder is silently dropped by the other path.
280+
- Nothing to add on the facade side: `cf.clusters.create()` takes a
281+
`ClusterConfiguration` rather than `**kwargs`, so a new field reaches it for
282+
free (RHOAIENG-98954).
283+
- `__post_init__` is **not idempotent** — it merges the default accelerator
284+
mapping into `extended_resource_mapping` and then rejects the merged result.
285+
`dataclasses.replace()` on a configured instance therefore raises. Use
286+
`copy.copy()` and set the attribute, as `ClusterHandler.create` does when it
287+
injects the namespace.
274288
- Tests: `src/codeflare_sdk/ray/cluster/test_config.py` — see `test_config_creation_all_parameters`
275289
and `test_autoscaling_config_valid` for the pattern.
276290

@@ -281,6 +295,13 @@ Real examples for the most common change types. Follow these patterns, not descr
281295
fixture from `src/codeflare_sdk/ray/rayjobs/test/conftest.py`.
282296
- Methods reaching the Kubernetes API need `@_bound_to_api_client` — see
283297
"Client isolation" above.
298+
- `cf.jobs.create()` / `cf.jobs.submit()` are `@overload`ed so that passing
299+
neither or both of `cluster_name` / `cluster_config` is a type error, not just
300+
a runtime `ValueError`. A new keyword argument goes in three places: the
301+
implementation signature, the `JobOptions` TypedDict, and the docstring. Both
302+
methods delegate to `JobHandler._build()`; they cannot call each other,
303+
because an overloaded method cannot satisfy its own overloads without
304+
re-narrowing the target.
284305

285306
### Adding unit and e2e tests
286307

‎README.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Kubernetes client, so clusters and jobs created from it keep using that client.
4040

4141
```python
4242
from kube_authkit import AuthConfig
43-
from codeflare_sdk import Codeflare, SDKConfig
43+
from codeflare_sdk import ClusterConfiguration, Codeflare, SDKConfig
4444

4545
# Option 1: Auto-detect authentication (kubeconfig or in-cluster service account)
4646
cf = Codeflare(config=SDKConfig(namespace='my-project'))
@@ -66,8 +66,9 @@ cf = Codeflare(config=SDKConfig(
6666
namespace='my-project',
6767
))
6868

69-
# Now create your cluster
70-
cluster = cf.clusters.create(name='my-cluster', num_workers=2)
69+
# Now create your cluster. Every SDK entrypoint that takes a cluster
70+
# description takes the same ClusterConfiguration object.
71+
cluster = cf.clusters.create(ClusterConfiguration(name='my-cluster', num_workers=2))
7172
cluster.apply()
7273
```
7374

‎demo-notebooks/guided-demos/6_single_entrypoint.ipynb‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"metadata": {},
4545
"outputs": [],
4646
"source": [
47-
"from codeflare_sdk import Codeflare, SDKConfig\n",
47+
"from codeflare_sdk import ClusterConfiguration, Codeflare, SDKConfig\n",
4848
"from kube_authkit import AuthConfig"
4949
]
5050
},
@@ -116,9 +116,10 @@
116116
"source": [
117117
"## Step 3: Create and bring up a Ray cluster\n",
118118
"\n",
119-
"Use `cf.clusters.create()` to build a `Cluster` object. All keyword arguments\n",
120-
"are forwarded to `ClusterConfiguration`. The namespace is inherited from\n",
121-
"`SDKConfig` unless you override it."
119+
"Use `cf.clusters.create()` to build a `Cluster` object from a\n",
120+
"`ClusterConfiguration` — the same object `cf.jobs` and `Cluster()` take. The\n",
121+
"namespace is inherited from `SDKConfig` unless the config or the call overrides\n",
122+
"it."
122123
]
123124
},
124125
{
@@ -129,20 +130,22 @@
129130
"outputs": [],
130131
"source": [
131132
"cluster = cf.clusters.create(\n",
132-
" name=\"entrypoint-demo\",\n",
133-
" num_workers=2,\n",
134-
" head_cpu_requests=\"500m\",\n",
135-
" head_cpu_limits=\"500m\",\n",
136-
" head_memory_requests=5,\n",
137-
" head_memory_limits=8,\n",
138-
" worker_cpu_requests=\"250m\",\n",
139-
" worker_cpu_limits=1,\n",
140-
" worker_memory_requests=4,\n",
141-
" worker_memory_limits=6,\n",
142-
" head_extended_resource_requests={\"nvidia.com/gpu\": 0},\n",
143-
" worker_extended_resource_requests={\"nvidia.com/gpu\": 0},\n",
144-
" write_to_file=False,\n",
145-
" # local_queue=\"your-local-queue\", # optional — auto-detected if omitted\n",
133+
" ClusterConfiguration(\n",
134+
" name=\"entrypoint-demo\",\n",
135+
" num_workers=2,\n",
136+
" head_cpu_requests=\"500m\",\n",
137+
" head_cpu_limits=\"500m\",\n",
138+
" head_memory_requests=5,\n",
139+
" head_memory_limits=8,\n",
140+
" worker_cpu_requests=\"250m\",\n",
141+
" worker_cpu_limits=1,\n",
142+
" worker_memory_requests=4,\n",
143+
" worker_memory_limits=6,\n",
144+
" head_extended_resource_requests={\"nvidia.com/gpu\": 0},\n",
145+
" worker_extended_resource_requests={\"nvidia.com/gpu\": 0},\n",
146+
" write_to_file=False,\n",
147+
" # local_queue=\"your-local-queue\", # optional — auto-detected if omitted\n",
148+
" )\n",
146149
")"
147150
]
148151
},
@@ -193,7 +196,21 @@
193196
"id": "m1b2c3d4",
194197
"metadata": {},
195198
"outputs": [],
196-
"source": "from codeflare_sdk import ClusterConfiguration\n\njob = cf.jobs.submit(\n name=\"entrypoint-demo-job\",\n entrypoint=\"python -c 'import ray; ray.init(); print(\\\"Hello from the Codeflare entrypoint!\\\")'\",\n cluster_config=ClusterConfiguration(\n head_memory_requests=6,\n head_memory_limits=8,\n num_workers=1,\n worker_cpu_requests=1,\n worker_cpu_limits=1,\n worker_memory_requests=4,\n worker_memory_limits=6,\n ),\n)"
199+
"source": [
200+
"job = cf.jobs.submit(\n",
201+
" name=\"entrypoint-demo-job\",\n",
202+
" entrypoint=\"python -c 'import ray; ray.init(); print(\\\"Hello from the Codeflare entrypoint!\\\")'\",\n",
203+
" cluster_config=ClusterConfiguration(\n",
204+
" head_memory_requests=6,\n",
205+
" head_memory_limits=8,\n",
206+
" num_workers=1,\n",
207+
" worker_cpu_requests=1,\n",
208+
" worker_cpu_limits=1,\n",
209+
" worker_memory_requests=4,\n",
210+
" worker_memory_limits=6,\n",
211+
" ),\n",
212+
")"
213+
]
197214
},
198215
{
199216
"cell_type": "code",

‎docs/api/public-surface.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@
1818
"list_all_clusters"
1919
]
2020
},
21+
"codeflare": {
22+
"module": "codeflare_sdk.codeflare",
23+
"symbols": [
24+
"Codeflare",
25+
"SDKConfig",
26+
"JobOptions"
27+
],
28+
"notes": "Single entrypoint. JobOptions is a TypedDict describing the shared keyword arguments of the cf.jobs.create()/submit() overloads; it carries no runtime behaviour."
29+
},
2130
"rayjobs": {
2231
"module": "codeflare_sdk.ray.rayjobs",
2332
"symbols": [

‎src/codeflare_sdk/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
from .common.widgets import view_clusters
1616

17-
from .codeflare import Codeflare, SDKConfig
17+
from .codeflare import Codeflare, SDKConfig, JobOptions
1818

1919
from .common.kueue import (
2020
list_local_queues,

0 commit comments

Comments
 (0)