Skip to content

feat: add single Codeflare entrypoint with handler pattern and remove legacy auth - #1091

Merged
pawelpaszki merged 18 commits into
mainfrom
single-entry
Sep 29, 2026
Merged

pawelpaszki merged 18 commits into
mainfrom
single-entry

Conversation

@szaher

@szaher szaher commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces Codeflare class as the single SDK entrypoint — authentication,
    namespace, retries, timeout, and logging configured once via SDKConfig
  • Adds cf.clusters.* and cf.jobs.* handler pattern (thin orchestrators
    that delegate to existing Cluster/RayJob classes)
  • Removes legacy auth classes (TokenAuthentication, KubeConfigFileAuthentication,
    Authentication, KubeConfiguration) — kube-authkit is now the only auth method
  • Updates all demo notebooks and E2E tests to use the new entrypoint

Usage

from codeflare_sdk import Codeflare, SDKConfig

cf = Codeflare(config=SDKConfig(
    auth=AuthConfig(method="auto"),
    namespace="my-project",
))

cluster = cf.clusters.create(name="train", num_workers=4)
cluster.apply()
job = cf.jobs.submit(name="train-job", entrypoint="python train.py")

Test plan

- 394 unit tests pass (32 new), 2 skipped
- 100% coverage on codeflare.py, 93% on auth.py
- Pre-commit (black, trailing whitespace, YAML) passes
- E2E tests updated — verify on OpenShift cluster
- Demo notebook 6_single_entrypoint.ipynb runs end-to-end on a real cluster

# What changes have been made
<!-- describe a summary of the change, add any additional motivation and context as needed -->

# Verification steps
<!-- Add thorough verification steps with sufficient level of detail for those without context to verify the change-->
<!-- AND Add thorough upgrade verification steps OR include a reason as to why it is not required -->
<!-- OR state "Not applicable" or "N/A" if your type of change doesn't require verification -->

## Checks
- [ ] I've made sure the tests are passing. 
- Testing Strategy
 - [ ] Unit tests
 - [ ] Manual tests
 - [ ] Testing is not required for this change

<!-- You can find out information on the review process at this link https://github.com/project-codeflare/codeflare/blob/develop/CONTRIBUTING.md#getting-feedback-on-your-contribution -->

@codecov

codecov Bot commented May 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.67456% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.50%. Comparing base (c662a25) to head (34b399f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/codeflare_sdk/ray/rayjobs/config.py 91.34% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1091      +/-   ##
==========================================
- Coverage   96.73%   96.50%   -0.24%     
==========================================
  Files          23       24       +1     
  Lines        2391     2319      -72     
==========================================
- Hits         2313     2238      -75     
- Misses         78       81       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2026
@kryanbeane

Copy link
Copy Markdown
Contributor

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2026
szaher and others added 11 commits September 11, 2026 12:01
Introduces the design for a unified SDK entrypoint with SDKConfig,
ClusterHandler, and JobHandler using kube-authkit for authentication.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ean set_api_client, update E2E/notebooks

- Remove print and verification call from set_api_client (now internal-only)
- Make namespace fallback consistent: all handler methods fall back to "default"
- Fix E2E tests: replace TokenAuthentication with Codeflare/SDKConfig
- Fix demo notebooks: use Codeflare(config=SDKConfig(auth=...)) instead of set_api_client
- Fix unused import (patch) and non-deterministic set ordering in error msg
- Add tests for namespace fallback in create() and submit()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Saad Zaher <szaher@redhat.com>
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2026
szaher and others added 3 commits September 11, 2026 12:12
The `list` method on ClusterHandler shadows the built-in `list` type,
causing mypy to interpret `-> list` on `list_queued` as a reference to
the method rather than the type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Saad Zaher <szaher@redhat.com>
Signed-off-by: Saad Zaher <szaher@redhat.com>
@szaher

szaher commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

/unhold

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2026

@pawelpaszki pawelpaszki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenShift / E2E status

E2E against OpenShift is passing on this branch: the kubernetes and kubernetes-rayjob workflows completed successfully, and the updated E2E paths (tests/e2e_v2/..., tests/e2e/support.py) use Codeflare + kube_authkit.AuthConfig instead of legacy TokenAuthentication. That gives good confidence for RHOAI/OpenShift token flows.

Unit tests, mypy, ruff, pre-commit, and codecov project checks are also green.


Summary

This PR introduces a solid Codeflare + SDKConfig entrypoint with cf.clusters / cf.jobs handlers, removes legacy auth in favor of kube-authkit, and consolidates RayJob embedded-cluster config onto ClusterConfiguration (RHAIENG-2063). The direction is right and the new test_codeflare.py coverage is strong.

Because this bundles multiple breaking API changes with subtle RayJob CR spec differences, I am requesting a few changes before merge (below). Non-blocking nits are called out separately.


What works well

  • Thin handlers delegating to existing Cluster / RayJob / list helpers — good separation without reimplementing lifecycle logic.
  • SDKConfig validation and handler tests are clear; TestLegacyAuthRemoved documents the new public contract.
  • Demo notebooks and E2E largely migrated to Codeflare(config=SDKConfig(auth=...)).
  • builtins.list return annotations on ClusterHandler.list / list_queued correctly avoid mypy shadowing issues.

Must change (blocking)

1. Flagship demo notebook imports removed ManagedClusterConfig

6_single_entrypoint.ipynb Step 4 still uses ManagedClusterConfig, which this PR removes. Users following the primary onboarding notebook will hit ImportError. Use ClusterConfiguration (and updated field names, e.g. head_extended_resource_requests instead of head_accelerators).

See inline comment on the notebook.

2. SDKConfig.retries / SDKConfig.timeout are documented but not applied

Both fields are validated and tested on the dataclass, but Codeflare.__init__ never consumes them. Either wire them through (K8s client / wait helpers) or remove from the public API in this PR to avoid misleading configuration.

See inline comment on codeflare.py.

3. Migration / upgrade documentation for breaking changes

This PR removes in one step:

Removed Replacement
TokenAuthentication, KubeConfigFileAuthentication, … kube_authkit.AuthConfig + Codeflare(SDKConfig(auth=...))
Top-level set_api_client, AuthConfig, get_k8s_client Codeflare init or codeflare_sdk.common.kubernetes_cluster.auth
ManagedClusterConfig ClusterConfiguration (+ field renames)

Please add a CHANGELOG entry and/or short migration section (before/after for token + kubeconfig, RayJob cluster_config, and export changes). The PR template verification / upgrade sections are still largely empty.

4. Document RayJob embedded-cluster spec deltas (RHAIENG-2063)

Consolidating on ClusterConfiguration + build_ray_cluster_spec() changes runtime specs vs old ManagedClusterConfig, including:

  • Different resource defaults (e.g. head CPU/memory).
  • enableInTreeAutoscaling now follows enable_autoscaling (default False — OK for Kueue today, but behavior changes if autoscaling is enabled).
  • autoscalerOptions always present in the built spec.
  • ODH CA volumes always merged for job-managed clusters.
  • Image pull policy IfNotPresent → Always for head/worker.
  • Worker container / group naming aligned with RayCluster builder (machine-learning, small-group-{name}).

Please call these out in release notes (and reference RHAIENG-2063). A golden-spec regression test would help if you have a prior baseline.

See inline comment on ray/rayjobs/config.py.


Non-blocking nits

  • SDKConfig.namespace docstring says "auto-detected if not set", but handlers fall back to the literal "default". Align docs or implement detection (e.g. from kube context). Inline on codeflare.py.
  • ClusterHandler.list shadows the builtin list; consider list_clusters() / list_all() in a follow-up for readability.
  • Global api_client: each Codeflare() calls set_api_client() — last instance wins in-process. Worth a one-line doc warning. Inline on Codeflare.__init__.
  • set_api_client no longer probes the API via get_api_group() — failures may surface later; acceptable if intentional. Inline on auth.py.
  • PR hygiene: contributor checkboxes still unchecked; codecov patch reports ~9 uncovered lines — quick pass welcome.
  • Scope: entrypoint + auth removal + config consolidation in one PR increases review/rollback cost; fine if intentional for one release, but migration docs become more important.

Verdict

Request changes — fix the demo notebook, resolve retries/timeout API surface, and add migration + RayJob spec notes. Happy to re-review once those are addressed.

Comment thread demo-notebooks/guided-demos/6_single_entrypoint.ipynb Outdated
Comment thread src/codeflare_sdk/codeflare.py Outdated
Comment thread src/codeflare_sdk/codeflare.py Outdated
Comment thread src/codeflare_sdk/codeflare.py
Comment thread src/codeflare_sdk/codeflare.py
Comment thread src/codeflare_sdk/common/kubernetes_cluster/auth.py
Comment thread src/codeflare_sdk/ray/rayjobs/config.py
Comment thread src/codeflare_sdk/__init__.py
Signed-off-by: Saad Zaher <szaher@redhat.com>
@pawelpaszki

pawelpaszki commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up: RayJob config migration doc

Thanks for the updates in fix review comments — the demo notebook, SDKConfig cleanup, and docs/auth_migration_guide.md address the earlier feedback well.

One small docs item remains: auth_migration_guide.md links to docs/rayjob_config_migration_guide.md, but that file is not on the branch yet, so the links are currently broken. A short guide covering the RHAIENG-2063 move from ManagedClusterConfig → ClusterConfiguration (field renames, default resource differences, and embedded RayJob spec behavior such as autoscaling / ODH volumes / image pull policy) would close the loop for upgraders.

This can land post-merge as a follow-up PR (or a quick docs-only commit right after merge) — I would not block merge on it, but it would be good to track so release notes and the auth guide stay accurate.

Signed-off-by: Saad Zaher <szaher@redhat.com>
@pawelpaszki

Copy link
Copy Markdown
Contributor

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 29, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 29, 2026
@openshift-ci

openshift-ci Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pawelpaszki

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2026
@pawelpaszki
pawelpaszki merged commit a3a0598 into main Sep 29, 2026
30 of 31 checks passed
@pawelpaszki
pawelpaszki deleted the single-entry branch September 29, 2026 13:36
pawelpaszki pushed a commit that referenced this pull request Oct 6, 2026
…nt (#1176)

* RHOAIENG-98754: bind Codeflare instances to their own K8s client

Post-merge hardening of the Codeflare single entrypoint added in #1091.
This is PR 1 of a planned series; it covers client isolation and the
facade's job-submission contract.

P1 client isolation
-------------------
Codeflare.__init__ stored its client in module-level global state via
set_api_client(), while Cluster, RayJob and the list/get helpers all read
the global get_api_client(). Constructing a second Codeflare instance
therefore silently repointed objects created from the first one, so
operations could run against the wrong Kubernetes cluster.

Objects now carry their own client. auth.py gains a ContextVar plus a
_use_api_client() context manager and a _bound_to_api_client decorator;
get_api_client() prefers the context-scoped client and falls back to the
module global. Cluster, RayJob, get_cluster, list_all_clusters and
list_all_queued take an additional api_client argument, and the facade
handlers pass the instance's own client explicitly.

A ContextVar is used rather than threading a client parameter through
every call site because the client is resolved in ~32 places across
build_ray_cluster, the Kueue helpers and cert generation. This keeps the
change contained and leaves those helpers untouched.

P1 cf.jobs.submit() contract
----------------------------
The advertised cf.jobs.submit(name=..., entrypoint=...) could never work:
RayJob requires exactly one of cluster_name/cluster_config. Existing tests
missed this because they mock the RayJob class wholesale. cluster_name and
cluster_config are now explicit keyword-only parameters on both submit()
and create(), so the requirement is visible to type checkers and IDEs, and
the error raised when neither is supplied no longer says "but not both".

P2 namespace resolution
-----------------------
The facade resolved namespaces with `or "default"`, silently targeting the
default namespace and defeating RayJob's own context detection. A single
_resolve_namespace() helper now applies: explicit argument, then
SDKConfig.namespace, then the detected current namespace, then a clear
error.

Backward compatibility
----------------------
api_client is additive and defaults to None, which preserves the existing
global-client behaviour for direct callers such as Cluster(config) and
RayJob(...). Codeflare still calls set_api_client() so those callers are
unaffected. No public argument became mandatory and no field was renamed.

Behaviour changes, both facade-only:
- the facade no longer falls back to the "default" namespace
- the neither-target error message changed

Testing
-------
New src/codeflare_sdk/test_client_isolation.py covers client isolation
(including that a second instance cannot steal the first's client and that
the scoped client does not leak past an operation), legacy global
fallback, the submit contract, and namespace precedence.

Pre-existing failures unrelated to this change, also failing on main:
test_kueue.py::test_cluster_creation_local_queue and
test_config.py::test_default_cluster_creation, both caused by Python 3.13
having no default Ray image in this environment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98754: apply ruff-format from pre-commit

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98754: address PR #1176 review

Keep namespace positional on the job handlers
---------------------------------------------
Moving namespace behind * made positional submit(name, entrypoint, ns)
raise TypeError, which is a facade API break that was not classified.
namespace is back in its original third slot; only the newly added
cluster_name/cluster_config are keyword-only, so no previously valid call
changes meaning. The handlers still accept every RayJob parameter, so
nothing was lost when **kwargs was replaced.

Make config_check() honour the scoped client
--------------------------------------------
config_check() keyed off the module global only. With a scoped client
active but no global set, it fell through to kube-authkit auto-detection
and assigned the module-level api_client as a side effect of a scoped
call. It now returns early when a scoped client is active.

Scope the remaining Cluster entry points
----------------------------------------
Cluster.config_check() and the Cluster._client_headers property resolve a
client but were not decorated. Both now carry @_bound_to_api_client. An
audit over both classes confirms no undecorated client resolver remains:
29 Cluster methods and 13 RayJob methods, zero undecorated. The module
level functions in cluster.py that resolve a client are either scoped
(get_cluster) or internal helpers only reachable from an already scoped
caller.

Assert RayJob isolation at the API-call level
---------------------------------------------
job._api_client is client_a only proves the attribute was stored. Added
the same post-cf_b API-call assertion the Cluster path already had, via
_build_rayjob_cr() which resolves the client through the Kueue helpers.
This behaviour was already correct; the test is a regression guard.

One existing test in this file passed for the wrong reason: a stale
module-level config_path leaked from an earlier test and short-circuited
config_check(). The autouse fixture now restores config_path as well as
api_client.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98754: review nits from b6b184b

Scope two more Cluster members
------------------------------
_client_verify_tls reaches a client transitively via
_is_openshift_cluster(), which the previous audit missed because it only
looked for a literal get_api_client() in the method body. Reached
directly rather than through a decorated caller, it could probe the
wrong cluster after a second Codeflare exists. Now decorated, with a
test that asserts the ApisApi call uses the owning client.

cluster_uri() resolves no client itself — it calls the already decorated
_check_tls_certs_exist() — but it is a public entry point, so decorating
it keeps the rule uniform at no behavioural cost.

Type runtime_env properly
-------------------------
Optional[Union[Dict[str, Any], Any]] collapses to Any and documents
nothing. Now Optional[Union[RuntimeEnv, Dict[str, Any]]], matching the
RayJob constructor, with a test asserting the two stay equal.

Docstring corrections
---------------------
SDKConfig.namespace still promised a 'default' fallback the facade no
longer has. config_check()'s priority list started at the module global
and omitted the scoped client the code now checks first.

Coverage for the module-helper path
-----------------------------------
clusters.list() and clusters.list_queued() route through the module
helpers and the ContextVar rather than Cluster._api_client, and had no
two-client test. Added. Also added one asserting the CustomObjectsApi
RayJob captures at init is built on its own client. Both passed on first
run: these were coverage gaps, not bugs.

The autouse fixture now asserts the ContextVar is clear after each test.
_use_api_client's finally already guarantees this; the assertion stops a
leaked scope from silently greening a later test, which is the failure
mode config_path actually hit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
pawelpaszki pushed a commit that referenced this pull request Oct 6, 2026
)

* RHOAIENG-98754: fix stale public-API references in demos and docs

Part of P1-6. PR #1091 removed `ManagedClusterConfig` and the top-level
`set_api_client` export, but the guided demos and user docs still import
them. Every affected example fails at the import cell, so the two guided
notebook CI jobs (`verify-5_submit_rayjob_cr`, `verify-6_autoscaling`)
are red on `main` and the README quickstart does not run.

Notebooks:
- 5_submit_rayjob_cr: `ManagedClusterConfig` -> `ClusterConfiguration`,
  `head_accelerators`/`worker_accelerators` ->
  `head_extended_resource_requests`/`worker_extended_resource_requests`.
- 6_autoscaling: drop `set_api_client`/`get_k8s_client`, authenticate via
  `Codeflare(SDKConfig(auth=...))`.
- 7_rayjob_checkpointing_example: same two fixes. Not covered by CI, but
  broken in the same way. `verify_ssl` is now set on `cf.client`.

Docs:
- README quickstart rewritten onto `Codeflare`/`SDKConfig`.
- authentication.rst: all five auth methods rewritten onto the entrypoint.
  The "Deprecated Authentication Methods" section claimed
  `TokenAuthentication`/`KubeConfigFileAuthentication` "remain functional";
  they raise `ImportError`. Corrected to "Removed".
- rayjob.rst: `ManagedClusterConfig` -> `ClusterConfiguration`.
- AGENTS.md and the rayjobs rule files pointed at a
  `ManagedClusterConfig` dataclass that no longer exists;
  `rayjobs/config.py` is now the `build_ray_cluster_spec` builder.

Verified by applying the workflow's jq/sed transformations to copies of
the notebooks and checking every `codeflare_sdk` import resolves. The jq
cell-deletion step also requires `source` to stay a list of lines, so the
notebooks keep their original JSON formatting.

Not included, deliberately:
- `docs/api/public-surface.json` still lists `Authentication`,
  `KubeConfiguration`, `TokenAuthentication` and
  `KubeConfigFileAuthentication`, which no longer exist. Removing them
  pre-empts the pending maintainer decision on whether to restore the
  legacy auth shims, so they are left for that discussion.
- CI drift enforcement for the public API surface.
- `docs/rayjob_config_migration_guide.md` names `ManagedClusterConfig` by
  design, as the "before" side of the migration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98754: bring AGENTS.md up to date with the current SDK

AGENTS.md (symlinked as CLAUDE.md) had drifted from the code after the
single-entrypoint and client-isolation work.

Corrected:
- `RayJob` cited at line 58; it is at 64.
- "8 notebooks"; there are 9 — `6_autoscaling` and `6_single_entrypoint`
  share the `6_` prefix.
- `.cursor/rules/` listed 6 of 10 files, and claimed `.claude/rules/`
  mirrors all of them. It mirrors only the three path-scoped rules; the
  numbered 01-07 rules exist only under `.cursor/rules/`.

Added, because their absence leads to wrong code rather than just
missing context:
- The `Codeflare` entrypoint, with a worked example and a pointer to
  `src/codeflare_sdk/codeflare.py`. It was not mentioned at all.
- Client isolation: `@_bound_to_api_client` on `Cluster`/`RayJob`
  methods, the `api_client=None` parameter on module-level helpers, and
  the ContextVar resolution order. Omitting the decorator is silent —
  the method works until a second `Codeflare` exists, then talks to the
  wrong cluster — so "Kubernetes API Patterns" as written was a trap.
- `WorkerGroup`, and the warning that `build_ray_cluster.py` and
  `rayjobs/config.py` are two builders over one dataclass: a field added
  to only one is silently dropped by the other path.
- Guided notebook jobs are gated on the `test-guided-notebooks` label
  and otherwise report skipped, so a broken notebook looks green. Also
  the `jq` cell-deletion step requires list-of-lines `source`.
- `mypy` and `lint-imports` commands, and the ruff version mismatch that
  makes the CI `precommit` job fail with "files were modified by this hook".
- `public-surface.json` is not CI-checked and currently lists four
  removed auth symbols.

Every path and cited line number in the file was verified against the
tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98754: make `cf` actually own the client in notebooks 6 and 7

Review follow-up. Both notebooks constructed a `Codeflare` and then
ignored it: `Cluster(...)`, `RayJob(...)` and `get_cluster(...)` resolved
their client from the module-level global instead. That works only
because `Codeflare.__init__` still calls `set_api_client()` — the
last-writer-wins global #1176 exists to move off. Notebook 7's markdown
claimed the isolation the code did not do.

Notebook 7 (not covered by any CI workflow, so `cf` always exists):
- `RayJob(..., api_client=cf.client)`
- `get_cluster(..., api_client=cf.client)` at all three call sites

Notebook 6 needs one extra step: the guided-notebook workflow deletes
its auth cell (it matches the "Authenticate to your Kubernetes/OpenShift
cluster using kube-authkit" marker), so `cf` does not exist under CI and
`api_client=cf.client` would be a NameError. Instead `CF_CLIENT` is
seeded to None in the imports cell and set from `cf.client` in the auth
cell:

- `Cluster(ClusterConfiguration(...), api_client=CF_CLIENT)`
- `CoreV1Api(CF_CLIENT or get_api_client())` in both pod-counting helpers

Users get an explicitly bound client; CI keeps today's behaviour, since
`api_client=None` is the legacy fallback. The shared constants moved to
the imports cell so `NAMESPACE` is available to `SDKConfig`.

A full rewrite onto `cf.clusters.create(...)` / `cf.jobs.submit(...)` is
deliberately left out to avoid churning guided-notebook CI in this PR.

Verified by replaying the workflow's jq/sed steps: after the auth cell is
deleted, `cf` appears in no executable statement (AST-checked, not grep —
it survives in a comment), `CF_CLIENT` is None, and every sed still
matches. Separately, binding was checked functionally: a cluster built
with `api_client=cf.client` still issues its API calls on that client
after a second `Codeflare` takes over the global.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
szaher added a commit that referenced this pull request Oct 7, 2026
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>
pawelpaszki pushed a commit that referenced this pull request Oct 7, 2026
* 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>
szaher added a commit that referenced this pull request Oct 7, 2026
…rule

docs/raycluster_spec_parity.md is the audit the ticket asks for: all 36
ClusterConfiguration fields classified SUPPORTED (29), CONTEXT_ONLY (3),
NOT_APPLICABLE (3) or CONFLICTED (1), each with a reason. The page is not
the enforcement — ray/test_builder_parity.py is — but it explains the
decisions the test encodes, including why raycluster_spec.py has to stay
free of Kubernetes API calls.

The rayjobs rule files claimed "Must NOT import from ray.cluster or
ray.client", which stopped being true at #1091: rayjobs/config.py:46 and
rayjob.py:36 both import ClusterConfiguration from ray.cluster.config, and
.importlinter has no contract for it, so CI never caught the contradiction.
Both files now describe the real boundary — cluster.config and
cluster.raycluster_spec are allowed, everything else in ray.cluster is not,
and it is prose-only — and point at the shared module as the place to add a
field.

AGENTS.md's two-builder hazard note said a field added to one builder is
dropped by the other, with no mention of where to put one instead. It now
names raycluster_spec.py, the parity test and the audit page, and carries
the no-I/O constraint.

Every repo path and symbol cited across the four files was checked to
resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
szaher added a commit that referenced this pull request Oct 9, 2026
…rule

docs/raycluster_spec_parity.md is the audit the ticket asks for: all 36
ClusterConfiguration fields classified SUPPORTED (29), CONTEXT_ONLY (3),
NOT_APPLICABLE (3) or CONFLICTED (1), each with a reason. The page is not
the enforcement — ray/test_builder_parity.py is — but it explains the
decisions the test encodes, including why raycluster_spec.py has to stay
free of Kubernetes API calls.

The rayjobs rule files claimed "Must NOT import from ray.cluster or
ray.client", which stopped being true at #1091: rayjobs/config.py:46 and
rayjob.py:36 both import ClusterConfiguration from ray.cluster.config, and
.importlinter has no contract for it, so CI never caught the contradiction.
Both files now describe the real boundary — cluster.config and
cluster.raycluster_spec are allowed, everything else in ray.cluster is not,
and it is prose-only — and point at the shared module as the place to add a
field.

AGENTS.md's two-builder hazard note said a field added to one builder is
dropped by the other, with no mention of where to put one instead. It now
names raycluster_spec.py, the parity test and the audit page, and carries
the no-I/O constraint.

Every repo path and symbol cited across the four files was checked to
resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
szaher added a commit that referenced this pull request Oct 9, 2026
This branch now changes no cluster or job behaviour. It shares duplicated
spec-building code and records the differences; the two behavioural fixes
have moved out.

GCS fault tolerance is no longer emitted on the RayJob path. Investigating
the release risk turned up 52a351a, "RHOAIENG-30720: Remove GCS FT for
Lifecycled RayClusters", which stripped those four fields from the
lifecycled path in August 2025 because the feature did not work — head pod
restarts lost state — and whose ticket says the RayJob implementation was
out of scope for the fix. So the absence was a decision, not drift, and
emitting them here would re-enable something nobody has validated on that
path. The real bug #1091 introduced is narrower: making
ClusterConfiguration shared means the fields are now accepted and validated
on the RayJob path and then ignored. Rejecting them or emitting them is
RHOAIENG-98943's call; the asymmetry is pinned by
test_gcs_fault_tolerance_is_standalone_only so neither side moves by
accident.

config.labels on the head and default worker pods moves to #1184, two lines
that can ship without this refactor.

labels is reclassified PARTIAL rather than a gap. A presence check passes on
both paths because the RayJob builder merges config.labels into additional
worker group pods; what differs is the default head and worker pods. The
earlier xfail would have XPASSed once the sentinel config gained a real
WorkerGroup, which is how this surfaced.

docs/raycluster_spec_parity.md gains a PENDING DECISION section and the
counts still total 36: SUPPORTED 24, PARTIAL 1, PENDING DECISION 4,
CONTEXT_ONLY 3, NOT_APPLICABLE 3, CONFLICTED 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
szaher added a commit that referenced this pull request Oct 9, 2026
…rule

docs/raycluster_spec_parity.md is the audit the ticket asks for: all 36
ClusterConfiguration fields classified SUPPORTED (29), CONTEXT_ONLY (3),
NOT_APPLICABLE (3) or CONFLICTED (1), each with a reason. The page is not
the enforcement — ray/test_builder_parity.py is — but it explains the
decisions the test encodes, including why raycluster_spec.py has to stay
free of Kubernetes API calls.

The rayjobs rule files claimed "Must NOT import from ray.cluster or
ray.client", which stopped being true at #1091: rayjobs/config.py:46 and
rayjob.py:36 both import ClusterConfiguration from ray.cluster.config, and
.importlinter has no contract for it, so CI never caught the contradiction.
Both files now describe the real boundary — cluster.config and
cluster.raycluster_spec are allowed, everything else in ray.cluster is not,
and it is prose-only — and point at the shared module as the place to add a
field.

AGENTS.md's two-builder hazard note said a field added to one builder is
dropped by the other, with no mention of where to put one instead. It now
names raycluster_spec.py, the parity test and the audit page, and carries
the no-I/O constraint.

Every repo path and symbol cited across the four files was checked to
resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
szaher added a commit that referenced this pull request Oct 9, 2026
This branch now changes no cluster or job behaviour. It shares duplicated
spec-building code and records the differences; the two behavioural fixes
have moved out.

GCS fault tolerance is no longer emitted on the RayJob path. Investigating
the release risk turned up 52a351a, "RHOAIENG-30720: Remove GCS FT for
Lifecycled RayClusters", which stripped those four fields from the
lifecycled path in August 2025 because the feature did not work — head pod
restarts lost state — and whose ticket says the RayJob implementation was
out of scope for the fix. So the absence was a decision, not drift, and
emitting them here would re-enable something nobody has validated on that
path. The real bug #1091 introduced is narrower: making
ClusterConfiguration shared means the fields are now accepted and validated
on the RayJob path and then ignored. Rejecting them or emitting them is
RHOAIENG-98943's call; the asymmetry is pinned by
test_gcs_fault_tolerance_is_standalone_only so neither side moves by
accident.

config.labels on the head and default worker pods moves to #1184, two lines
that can ship without this refactor.

labels is reclassified PARTIAL rather than a gap. A presence check passes on
both paths because the RayJob builder merges config.labels into additional
worker group pods; what differs is the default head and worker pods. The
earlier xfail would have XPASSed once the sentinel config gained a real
WorkerGroup, which is how this surfaced.

docs/raycluster_spec_parity.md gains a PENDING DECISION section and the
counts still total 36: SUPPORTED 24, PARTIAL 1, PENDING DECISION 4,
CONTEXT_ONLY 3, NOT_APPLICABLE 3, CONFLICTED 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pawelpaszki pushed a commit that referenced this pull request Oct 12, 2026
…ge) (#1182)

* RHOAIENG-98942: add a builder parity test before unifying the two paths

ClusterConfiguration feeds two independent spec builders — the standalone
RayCluster in ray/cluster/build_ray_cluster.py and the embedded
rayClusterSpec in ray/rayjobs/config.py. A field wired into one and
forgotten in the other raises nothing; the cluster just comes up without
it. Unifying them is a large refactor, so the parity check lands first and
the refactor is held to it.

The test sets every ClusterConfiguration field to a recognisable sentinel,
renders both specs and asserts each sentinel reaches both. Anything that
legitimately does not reach a builder is listed with a reason:

- CONTEXT_ONLY (3): overwrite_default_resource_mapping and
  enable_usage_stats are consumed by __post_init__ (the latter flows on via
  envs); verify_tls governs dashboard calls, not the CR.
- NOT_APPLICABLE (3): name, namespace and write_to_file are meaningless
  inside a RayJob, which derives '<job_name>-cluster' and embeds a spec
  with no metadata block.
- CONFLICTED (1): local_queue is set on both sides with undefined
  precedence (RHOAIENG-98949).

test_every_field_is_classified fails on a new field that is neither given a
sentinel nor classified, so "no field may be silently ignored" is enforced
rather than asserted.

Four fields the RayJob path drops today are xfail(strict=True) rather than
exempt, since they are bugs and the unification has to remove them:

- redis_address, redis_password_secret, external_storage_namespace —
  GCS fault tolerance is entirely absent from the RayJob builder
  (RHOAIENG-98943).
- labels — reaches additional worker groups only; the head and default
  worker pod templates at rayjobs/config.py:146,166 pass annotations and no
  labels. Not previously tracked by any ticket.

Also recorded in the test: build_ray_cluster() is not pure. It reaches the
Kubernetes API three times while rendering — local_queue_exists(),
get_default_local_queue() and validate_autoscaling_with_kueue() — which the
RayJob builder never does. The canonical builder therefore has to split a
pure renderer from the resolve-cluster-facts step, or the RayJob path
inherits network calls it does not make today.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: extract the shared RayCluster spec pieces

The standalone and RayJob builders each carried their own copy of the same
primitives, which is how they drifted in the first place. Both now delegate
to ray/cluster/raycluster_spec.py.

Proven identical to both originals before being removed: the ODH trusted-CA
volumes and mounts (byte-for-byte duplicates), cpu_limit_to_num_cpus (a
literal copy), the storage merge, resource requirements, GPU counting,
extended-resource mapping, and the head and worker containers — the last two
compared by repr against both implementations.

The module is deliberately side-effect-free. build_ray_cluster() reaches the
Kubernetes API three times while rendering — local_queue_exists(),
get_default_local_queue() and validate_autoscaling_with_kueue() — and the
RayJob builder makes no calls at all, so anything needing the cluster stays
in the caller. Lifting that I/O out is the next step.

Two pieces are added here for the fixes that follow rather than used yet:
gcs_fault_tolerance_options() and worker_replica_counts(), both unit tested.

build_ray_cluster.py keeps its existing function names as one-line
delegations, so no call site moved and the diff stays reviewable. Two spec
builders totalling 1,186 lines are now 838 plus a 279-line shared module.

Test patch targets for update_image moved with it, except in
test_build_spec_additional_group_inherits_image: additional worker groups
resolve their own image in rayjobs/config.py and still patch there.

Not yet fixed, still xfail: the five fields the RayJob path drops.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: stop the RayJob path dropping GCS FT and labels

The five fields the parity test had marked xfail now reach both builders,
so KNOWN_GAPS is empty and the xfails are gone.

GCS fault tolerance (RHOAIENG-98943) was the dangerous one. enable_gcs_ft,
redis_address, redis_password_secret and external_storage_namespace were
accepted and validated by ClusterConfiguration, then ignored entirely by
the RayJob builder — the four names did not appear anywhere in
rayjobs/config.py. A job configured for fault tolerance came up with no
Redis to recover from, reported nothing, and only failed when the head node
died. build_ray_cluster_spec() now emits gcsFaultToleranceOptions from the
shared helper, the same one the standalone path uses.

labels reached additional worker groups only. _build_additional_worker_group_spec
merged config.labels into its pod template while the head and default worker
templates passed annotations and nothing else, so ClusterConfiguration(labels=...)
landed on extra worker groups and nowhere else. Both templates now pass them.

The asymmetry that remains is deliberate: the standalone path also puts
labels on the RayCluster metadata, which the embedded spec has no room for.
Pod templates are where they can land here, and where this path already put
them for additional worker groups.

Regression tests cover the RayJob path directly rather than relying on the
parity markers, including that metadata stays None when no labels are set.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: record the field audit and correct the stale rayjobs rule

docs/raycluster_spec_parity.md is the audit the ticket asks for: all 36
ClusterConfiguration fields classified SUPPORTED (29), CONTEXT_ONLY (3),
NOT_APPLICABLE (3) or CONFLICTED (1), each with a reason. The page is not
the enforcement — ray/test_builder_parity.py is — but it explains the
decisions the test encodes, including why raycluster_spec.py has to stay
free of Kubernetes API calls.

The rayjobs rule files claimed "Must NOT import from ray.cluster or
ray.client", which stopped being true at #1091: rayjobs/config.py:46 and
rayjob.py:36 both import ClusterConfiguration from ray.cluster.config, and
.importlinter has no contract for it, so CI never caught the contradiction.
Both files now describe the real boundary — cluster.config and
cluster.raycluster_spec are allowed, everything else in ray.cluster is not,
and it is prose-only — and point at the shared module as the place to add a
field.

AGENTS.md's two-builder hazard note said a field added to one builder is
dropped by the other, with no mention of where to put one instead. It now
names raycluster_spec.py, the parity test and the audit page, and carries
the no-I/O constraint.

Every repo path and symbol cited across the four files was checked to
resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: address review — use the shared helpers on both paths

Review from @pawelpaszki on #1182. All points verified against the code
before changing anything; every one of them held.

Blocking:

- The standalone builder still inlined the GCS fault tolerance block instead
  of calling the helper extracted for it, which is exactly the duplication
  98942/98943 were meant to remove. It now calls
  gcs_fault_tolerance_options(). The uncovered line Codecov flagged in
  build_ray_cluster.py was indeed that leftover ValueError.
- worker_replica_counts() was dead — only its own tests called it. Both
  builders now use it, replacing two copies of the replicas / minReplicas /
  maxReplicas branch.
- format_resources_param() was used on the RayJob side only; the standalone
  path inlined json.dumps. It now calls the shared one.

Non-blocking:

- The parity test claimed more than it checked. Fields whose MARKERS entry
  was None were filtered out of the parametrized test entirely, so
  num_workers, min_workers, max_workers, enable_autoscaling, enable_gcs_ft
  and additional_worker_groups were never parity-checked, and the
  additional_worker_groups sentinel was an empty list. Those fields are now
  listed in STRUCTURAL and asserted against the sanitised spec dicts rather
  than by substring on repr(), and the sentinel is a real WorkerGroup.
  Mutation-checked: dropping GCS FT or additional worker groups from the
  RayJob path fails the suite.
- Bottom-of-file imports moved to the top; there was no cycle, so the
  noqa: E402 pair was covering nothing.
- Deleted the ODH volumes section heading left stranded above
  build_ray_cluster_spec.
- docs: labels moved out of SUPPORTED into a PARTIAL section. Both paths
  carry them, but only the RayJob path puts them on the default head and
  worker pods, so a NetworkPolicy selector still behaves differently.
  Fixing that changes standalone output, so it belongs with the follow-up.
- docs: added an explicit list of what is shared versus what each builder
  still assembles independently.

The remaining blocking point — that the tree is still duplicated and 98942
should not close as done — is answered in the PR description rather than
here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: use format_resources_param for standalone extra groups

Review nit from @pawelpaszki: _build_worker_group_spec still inlined
json.dumps for its rayStartParams resources while every other call site
used the shared helper. The remaining json.dumps in this module is the
YAML file writer, which is unrelated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: split out the behaviour changes, keep consolidation only

This branch now changes no cluster or job behaviour. It shares duplicated
spec-building code and records the differences; the two behavioural fixes
have moved out.

GCS fault tolerance is no longer emitted on the RayJob path. Investigating
the release risk turned up 52a351a, "RHOAIENG-30720: Remove GCS FT for
Lifecycled RayClusters", which stripped those four fields from the
lifecycled path in August 2025 because the feature did not work — head pod
restarts lost state — and whose ticket says the RayJob implementation was
out of scope for the fix. So the absence was a decision, not drift, and
emitting them here would re-enable something nobody has validated on that
path. The real bug #1091 introduced is narrower: making
ClusterConfiguration shared means the fields are now accepted and validated
on the RayJob path and then ignored. Rejecting them or emitting them is
RHOAIENG-98943's call; the asymmetry is pinned by
test_gcs_fault_tolerance_is_standalone_only so neither side moves by
accident.

config.labels on the head and default worker pods moves to #1184, two lines
that can ship without this refactor.

labels is reclassified PARTIAL rather than a gap. A presence check passes on
both paths because the RayJob builder merges config.labels into additional
worker group pods; what differs is the default head and worker pods. The
earlier xfail would have XPASSed once the sentinel config gained a real
WorkerGroup, which is how this surfaced.

docs/raycluster_spec_parity.md gains a PENDING DECISION section and the
counts still total 36: SUPPORTED 24, PARTIAL 1, PENDING DECISION 4,
CONTEXT_ONLY 3, NOT_APPLICABLE 3, CONFLICTED 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: give every WorkerGroup field a parity sentinel

Review from @chipspeak: the WorkerGroup sentinel set only group_name and
replicas, so envs, labels, tolerations, image, the GPU settings and
extended resources could all drift between the two builders while this
suite stayed green. Correct — additional_worker_groups is assembled by a
second pair of duplicated functions, _build_worker_group_spec and
_build_additional_worker_group_spec, which can diverge independently of
the main builders this file was written to guard.

All 15 fields now carry sentinels, and the four inherit-or-override
rules get named tests on both paths, because a presence check cannot
express them: image and tolerations are overrides that fall back to the
cluster value, while envs and labels are merges where the group wins a
key collision. The inherit case is asserted separately from the override
case, so dropping either direction fails.

test_every_worker_group_field_is_classified mirrors the existing
ClusterConfiguration gate: a new WorkerGroup field must be given a
sentinel or routed to a named test via WORKER_GROUP_STRUCTURAL.

Also recorded in AGENTS.md, both rayjobs rule files and the audit doc,
as asked.

Mutation-checked, six ways: dropping the cluster-env merge, dropping the
cluster-label merge, ignoring wg.tolerations, ignoring wg.image,
dropping wg extended resources, and adding an unclassified field each
fail the suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* RHOAIENG-98942: make the parity doc read as reference, not changelog

Review from @chipspeak: either move it under docs/designs/History or
drop the language about the previous implementation, since a doc that
narrates what changed has to be maintained as a changelog forever.

Keeping it in place and taking the second option. docs/designs/History
holds one superseded design doc, so it is an archive — and this page is
live: AGENTS.md sends contributors here before adding a field, and
test_builder_parity.py names it in a failure message. Filing it under
History would make both pointers misleading.

So the framing is now current-state. Dropped "they were written
separately and drifted", "now live in", "the subject of the follow-up
to this work" and the @-attribution, retitled off "audit", and led with
what the reader needs: read this before adding a field, here is where
each one lands, here is which asymmetries are deliberate. The
duplicated assemblers are stated as a present constraint with the
ticket that tracks them rather than as history.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Pat O'Connor <114754356+chipspeak@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants