fix(backend): show a friendly error when KFP is unreachable

[Bhavd33p]

Summary

• When the KFP API server is unreachable (wrong host, server down, connection refused, etc.), the RPC layer used to let the raw urllib3.exceptions.MaxRetryError / requests.exceptions.ConnectionError bubble up, which the frontend displayed as an unhelpful generic error dialog.
• kale/rpc/kfp.py now catches those connection errors on every KFP-touching RPC entry point (list_experiments, get_ui_host, get_experiment, create_experiment, upload_pipeline, run_pipeline, get_run) and raises the existing (previously unused) RPCServiceUnavailableError, so the user instead sees:

  KFP is not reachable. You can still compile notebooks, but you cannot upload or run pipelines. You can find more information under <HOME>/kale.log

• No frontend changes were needed — _legacy_executeRpcAndShowRPCError in labextension/src/lib/RPCUtils.tsx already prefers err_details when showing the dialog.

Fixes #883

Test plan

• Added kale/tests/unit_tests/test_rpc_kfp.py covering MaxRetryError/ConnectionError handling on list_experiments, get_run, create_experiment, and confirming unrelated errors (e.g. ValueError) are not swallowed.
• uv run pytest kale/tests -vv — 254 backend tests pass (2 pre-existing/unrelated e2e failures reproduce identically on main, confirmed before this change).
• uv run ruff check kale and uv run ruff format --check kale pass.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stefanofioravanzo for approval. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[Bhavd33p]

@StefanoFioravanzo @ederign Kindly review the changes

[Copilot]

Pull request overview

Improves RPC feedback when the KFP API server is unreachable.

Changes:

• Converts KFP connection failures into actionable service-unavailable errors.
• Adds unit coverage for connection and unrelated errors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
kale/rpc/kfp.py	Adds centralized KFP connection-error handling.
kale/tests/unit_tests/test_rpc_kfp.py	Tests friendly error propagation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[harshhh817]

Not a maintainer, just a contributor passing through — this is a nice improvement, the unreachable-KFP dialog is genuinely unhelpful today. I checked the approach against the client internals and it holds up; one suggestion on test coverage.

The exception tuple is right. I was initially worried kfp_server_api would wrap connection failures into ApiException and the except would never fire, but its rest module only catches urllib3.exceptions.SSLError — MaxRetryError does propagate raw. So catching it here works.

The get_experiment(None, ...) → get_experiment(request, ...) change is necessary, not just tidying: now that get_experiment is decorated, the wrapper dereferences request.log / request.trans_id, so passing None would have turned a connection error into an AttributeError. Worth keeping in mind if any other internal call sites pass None.

Main suggestion — the tests may not cover the path users actually hit.

All four tests patch _get_client with side_effect=<connection error>, i.e. they assume the failure happens while constructing the client. But kfp.Client.__init__ only touches the network when the namespace is unset:

if not self._context_setting['namespace'] and self.get_kfp_healthz(...)

In the normal Kale setup the namespace is set (from the saved config or KALE_KFP_NAMESPACE), so _get_client() returns happily without contacting the server, and the MaxRetryError is raised later — by the API call itself (c.list_experiments()), inside the decorated function body.

The decorator wraps the whole body, so the fix still catches it and the behaviour is correct. But as written the tests only exercise the namespace-unset path, so they'd still pass if the decorator were only applied around client construction. Something like this would pin the common case:

def test_list_experiments_kfp_unreachable_on_api_call(_rpc_request):
    client = mock.MagicMock()
    client.list_experiments.side_effect = _max_retry_error()
    with (
        mock.patch("kale.rpc.kfp._get_client", return_value=client),
        pytest.raises(RPCServiceUnavailableError),
    ):
        kfp.list_experiments(_rpc_request)

Minor: worth a one-line comment that ping() is intentionally left undecorated (it already swallows everything and returns False) — otherwise it reads like an omission next to the other seven entry points.

Also 👍 on test_list_experiments_unrelated_error_not_swallowed — easy to forget, and exactly the regression that would bite later.

[Bhavd33p]

Thanks @harshhh817 — that was a useful read, both points are in (a393c56).

Tests now cover the path users actually hit. You were right that kfp.Client.__init__ only touches the network when the namespace is unset, so with a namespace configured the MaxRetryError comes out of the API call, not out of client construction. Added test_list_experiments_kfp_unreachable_on_api_call, test_get_run_kfp_unreachable_on_api_call and test_create_experiment_kfp_unreachable_on_api_call, which let _get_client() return a working client and make the API call raise. The original client-construction tests are kept, so both paths are pinned.

ping() now says in a comment that it is intentionally left undecorated, since it already reports an unreachable server by returning False instead of raising.

Copilot's note is covered too: test_create_experiment_kfp_unreachable returns a client on the first _get_client() call and fails the nested one, so execution really reaches get_experiment(request, ...) and a regression back to passing None would be caught.

Locally: 259 tests pass, ruff check clean, DCO green.

@StefanoFioravanzo @ederign this one is ready for review.

[ada333]

@Bhavd33p hi, could you please share some screen / video / steps to reproduce on how to get the error message? I was not able to see it when testing this PR

[jesuino]

I am not able to see this behavior possibly because of the issue created by Adam -> #948 - it would be good if you could address this on this PR as well so we can test all PR!

In any case, do we really need a decorator for this? Shouldn't it be enough to put an exception handler on kfp_client_factory.get_kfp_client when the Client is created? Locally I can see it throws an exception if it can't connect to KFP.

[jesuino]

Isn't this already logged every time a RPC function fails?