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

Update _wait_for_cluster_to_start func to check for libraries statuses #822

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

psivash
Copy link

@psivash psivash commented Oct 10, 2024

Resolves #821

Description

Enhance _wait_for_cluster_to_start to wait for library installation

Previously, the _wait_for_cluster_to_start function only checked if the cluster state was 'running'. This update adds a check to ensure all cluster libraries have been successfully installed (or skipped or restored) before proceeding. This ensures the cluster is fully ready for operations that depend on these libraries.

  • Added get_cluster_libraries_status to fetch the status of cluster libraries.
  • Updated _wait_for_cluster_to_start to wait until all libraries are in valid statuses ('INSTALLED', 'RESTORED', 'SKIPPED').

Checklist

  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

Enhance `_wait_for_cluster_to_start` to wait for library installation

Previously, the `_wait_for_cluster_to_start` function only checked if the cluster state was 'running'. This update adds a check to ensure all cluster libraries have been successfully installed (or skipped or restored) before proceeding. This ensures the cluster is fully ready for operations that depend on these libraries.

- Added `get_cluster_libraries_status` to fetch the status of cluster libraries.
- Updated `_wait_for_cluster_to_start` to wait until all libraries are in valid statuses ('INSTALLED', 'RESTORED', 'SKIPPED').
@psivash psivash changed the title Update _wait_for_cluster_to_start func to check for libraries statuses Update _wait_for_cluster_to_start func to check for libraries statuses Oct 10, 2024
return
else:
time.sleep(5)
libraries_status_response = self.get_cluster_libraries_status()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised this isn't done as part of 'STARTING'. Does every run need to wait for library installs, or just those that specify additional libraries?

Copy link
Author

Choose a reason for hiding this comment

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

Good question. I assume that basic default libraries are a part of 'STARTING' indeed and only the libraries that are added explicitly (the ones exposed via Libraries API) are not and need extra time so that python models do work correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If that's the case, let's take this branch only if additional libraries have been specified.

Copy link
Collaborator

@benc-db benc-db left a comment

Choose a reason for hiding this comment

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

Please see failing unit tests. Also, fyi, I'm going to be merging branch 1.9.latest into main today, so we'll probably need to rebase after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python models don't wait for all-purpose-cluster to be fully ready
2 participants