Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added timeout to w.clusters.ensure_cluster_running() #227

Merged
merged 7 commits into from
Aug 2, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Return ClusterDetails
  • Loading branch information
judahrand committed Jul 11, 2023
commit 3f04d7c1b8db1e3e282b12285bc364b64dfab082
13 changes: 5 additions & 8 deletions databricks/sdk/mixins/compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def select_node_type(self,
return nt.node_type_id
raise ValueError("cannot determine smallest node type")

def ensure_cluster_is_running(self, cluster_id: str) -> None:
def ensure_cluster_is_running(self, cluster_id: str) -> compute.ClusterDetails:
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need cluster details to be returned? this will have to be adjusted across all SDKs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't have to 🤷‍♂️ Just seemed more useful given that we have it than returning None.

Do you want me to revert that commit?

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@judahrand could you address this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry! I'd forgotten to follow up on this. Done now.

"""Ensures that given cluster is running, regardless of the current state"""
timeout = datetime.timedelta(minutes=20)
deadline = time.time() + timeout.total_seconds()
Expand All @@ -218,17 +218,14 @@ def ensure_cluster_is_running(self, cluster_id: str) -> None:
state = compute.State
info = self.get(cluster_id)
if info.state == state.RUNNING:
return
return info
elif info.state == state.TERMINATED:
self.start(cluster_id).result()
return
return self.start(cluster_id).result()
elif info.state == state.TERMINATING:
self.wait_get_cluster_terminated(cluster_id)
self.start(cluster_id).result()
return
return self.start(cluster_id).result()
elif info.state in (state.PENDING, state.RESIZING, state.RESTARTING):
self.wait_get_cluster_running(cluster_id)
return
return self.wait_get_cluster_running(cluster_id)
elif info.state in (state.ERROR, state.UNKNOWN):
raise RuntimeError(f'Cluster {info.cluster_name} is {info.state}: {info.state_message}')
except OperationFailed as e:
Expand Down