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

Installation checks if either configs ("enableProjectTypeInWorkspace", "enableWorkspaceFilesystem") are 'false' and makes them 'true' for valid installation #831

Closed
wants to merge 3 commits into from

Conversation

pritishpai
Copy link
Contributor

@pritishpai pritishpai commented Jan 22, 2024

Changes

Installation fails if either of the configs ("enableProjectTypeInWorkspace", "enableWorkspaceFilesystem") are 'false'. Checks the two configs via API and sets them to "true" if they are "false".

Linked issues

Resolves #576

Functionality

  • added relevant user documentation
  • added new CLI command
  • modified existing command: databricks labs ucx ...
  • added a new workflow
  • modified existing workflow: ...
  • added a new table
  • modified existing table: ...

Tests

  • manually tested
  • added unit tests
  • added integration tests
  • verified on staging environment (screenshot attached)

Using SDK for getting and setting conf values blocked by databricks/databricks-sdk-py#500

@pritishpai pritishpai requested review from a team and zpappa January 22, 2024 23:01
Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3a71dcf) 87.21% compared to head (b4a48d7) 87.23%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #831      +/-   ##
==========================================
+ Coverage   87.21%   87.23%   +0.02%     
==========================================
  Files          41       41              
  Lines        5200     5211      +11     
  Branches      936      938       +2     
==========================================
+ Hits         4535     4546      +11     
  Misses        458      458              
  Partials      207      207              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Use sdk, not api

src/databricks/labs/ucx/install.py Outdated Show resolved Hide resolved
src/databricks/labs/ucx/install.py Outdated Show resolved Hide resolved
@nfx
Copy link
Collaborator

nfx commented Feb 1, 2024

sdk is fixed - databricks/databricks-sdk-py#525

@pritishpai
Copy link
Contributor Author

The sdk changes are not released yet, so make changes to the toml file and pulled in latest commit. Can only merge after the next release. There are other changes in sdk that I had to make to update the codebase(e.g. [https://github.com/databricks/databricks-sdk-py/pull/522]). Should I added them as a part of this PR too @nfx ?

Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Could you implement it in the blueprint library instead? Eg manually create a new dummy workspace (for isolation), disable settings, then in Installation.load (or deeper), catch the "feature disabled" exception, call this code to set the status, and retry there.

This would be the most impactful implementation, that is the easiest to test in isolation

src/databricks/labs/ucx/install.py Show resolved Hide resolved
src/databricks/labs/ucx/install.py Show resolved Hide resolved
src/databricks/labs/ucx/install.py Show resolved Hide resolved
tests/integration/test_installation.py Show resolved Hide resolved
@pritishpai
Copy link
Contributor Author

Work now done in the blueprint repo:
databrickslabs/blueprint#42

@pritishpai
Copy link
Contributor Author

@pritishpai pritishpai closed this Feb 13, 2024
@pritishpai pritishpai deleted the fix/issue_576 branch February 13, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants