Improve KubeRayCluster cleanup

[photoroman]

Thanks for creating this amazing package! It's been super useful for running some of our ray workloads via dagster.

Summary:
Ensure that KubeRayCluster resources properly delete running ray clusters in case a dagster job fails with a BaseException type.

Issue:
I noticed that when a ray cluster has not started yet (e.g. because it is waiting for resources in a k8s cluster) and the dagster run that created it is terminated via UI, the ray cluster is not terminated. Resolve #96

Steps to reproduce:

• Start a dagster run that attempts to launch a ray cluster that remains pending (e.g. ask for more than the available resources). This will ensure that the try/except clause in yield_for_execution does not complete yet.
• Terminate the dagster run via dagster UI, which will trigger a DagsterExecutionInterruptedError, that inherits directly from BaseException.

Proposed Change:
Catch BaseException instead of Exception inside of yield_for_execution, because some of the dagster error types like e.g. DagsterExecutionInterruptedError inherit directly from BaseException and were therefore not caught before this change.

Verification:
I tested this in my dagster deployment. It's not clear to me how to write a unit test for it.

Open questions:
I saw that _maybe_cleanup_raycluster only runs the cleanup if the dagster status is != DagsterRunStatus.FAILURE. Why is that? Shouldn't the cluster be cleaned up in all cases, unless skip_cleanup=True?

[danielgafni]

Very cool, thanks for the PR! I've noticed this issue as well but never figured out the root case. I've modified your commit message to link this issue :)

I saw that _maybe_cleanup_raycluster only runs the cleanup if the dagster status is != DagsterRunStatus.FAILURE. Why is that? Shouldn't the cluster be cleaned up in all cases, unless skip_cleanup=True?

Just for debugging purposes, typically if a Run fails it's beneficial to be able to connect to the cluster. I typically have an automated cleanup schedule (see dagster_ray.kuberay.schedules) that eventually cleans up such clusters.

But I agree that this behavior is neither intuitive nor always desired. I'm open to introducing another option to configure it separately.

[photoroman]

Just for debugging purposes, typically if a Run fails it's beneficial to be able to connect to the cluster. I typically have an automated cleanup schedule (see dagster_ray.kuberay.schedules) that eventually cleans up such clusters.

Ah, makes sense. I'll use these schedules than.

But I agree that this behavior is neither intuitive nor always desired. I'm open to introducing another option to configure it separately.
Yes, I think it could be nice to clean up clusters by default, but have an option that allows to keep the cluster around for debugging if needed.