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

Remove failed libraries on running clusters #956

Merged
merged 1 commit into from
Dec 12, 2021
Merged

Remove failed libraries on running clusters #956

merged 1 commit into from
Dec 12, 2021

Conversation

nfx
Copy link
Contributor

@nfx nfx commented Dec 11, 2021

Whenever a library fails to get installed on a running cluster, we automatically remove it, so that the clean state of managed libraries is properly maintained. Without this fix users had to manually go to Clusters UI and remove library from a cluster, where it failed to install. Libraries add up to CREATE and UPDATE timeouts of databricks_cluster resource.

Fixes #599

Whenever a library fails to get installed on a running cluster, we automatically remove it, so that the clean state of managed libraries is properly maintained. Without this fix users had to manually go to Clusters UI and remove library from a cluster, where it failed to install. Libraries add up to CREATE and UPDATE timeouts of `databricks_cluster` resource.

Fixes #599
@nfx nfx requested a review from alexott December 11, 2021 15:09
@nfx nfx added this to the v0.4.1 milestone Dec 11, 2021
@codecov
Copy link

codecov bot commented Dec 11, 2021

Codecov Report

Merging #956 (bea055a) into master (ca42fef) will increase coverage by 0.03%.
The diff coverage is 95.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #956      +/-   ##
==========================================
+ Coverage   85.66%   85.69%   +0.03%     
==========================================
  Files         103      103              
  Lines        9060     9093      +33     
==========================================
+ Hits         7761     7792      +31     
- Misses        789      790       +1     
- Partials      510      511       +1     
Impacted Files Coverage Δ
libraries/libraries_api.go 96.07% <93.33%> (-0.87%) ⬇️
clusters/resource_cluster.go 78.97% <100.00%> (+1.25%) ⬆️

@nfx nfx added the suppress diff issue related to configuration drift, most likely from Cluster Manager label Dec 11, 2021
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

lgtm

Messages []string `json:"messages,omitempty"`
Library *Library `json:"library,omitempty"`
Status string `json:"status,omitempty"`
IsGlobal bool `json:"is_library_for_all_clusters,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

should we start deprecation of this flag? Because per documentation:

This option does not install the library on clusters running Databricks Runtime 7.0 and above.

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's not changeable by users anymore 🤷🏻‍♂️ but there still might be some workspaces with dbr7.0, that have this

@nfx nfx merged commit f3d0fad into master Dec 12, 2021
@nfx nfx deleted the issue/599 branch December 12, 2021 14:32
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this pull request Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suppress diff issue related to configuration drift, most likely from Cluster Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove managed library that failed to install on refresh
2 participants