-
Notifications
You must be signed in to change notification settings - Fork 90
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
Support joining an existing collection when installing UCX #1675
Merged
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
61c9692
initial draft of join collection
HariGS-DB d664da1
Merge branch 'main' into feature/collection
HariGS-DB e16b8cf
unit test cases
HariGS-DB 2fecd3d
formatting
HariGS-DB 18e8ea8
formatting
HariGS-DB c372a52
fixing unit test case issues
HariGS-DB 9abc2d8
fixing unit test case issues
HariGS-DB 66e5a55
redesigning methods
HariGS-DB 1b33340
redesigning methods
HariGS-DB 7ad9379
redesigning methods
HariGS-DB 71f434b
review comments
HariGS-DB 9470271
review comments
HariGS-DB 37513d6
review comments
HariGS-DB 69d71d9
review comments
HariGS-DB e10fea6
review comments
HariGS-DB 9cbbfd7
review comments
HariGS-DB 80f7c8c
test cases fixes
HariGS-DB dc9c88f
merging
HariGS-DB c8a24a5
Merge branch 'main' into feature/collection
HariGS-DB a393e55
merging
HariGS-DB 4cb7109
increasing coverage
HariGS-DB 933dee0
new collection logic
HariGS-DB 83e286c
int test
HariGS-DB 511ec6c
int test
HariGS-DB 49837a6
Merge branch 'main' into feature/collection
HariGS-DB a40c1bb
review comments
HariGS-DB 20177ee
review comments
HariGS-DB f9d883f
review comments and readme
HariGS-DB 2e3b6c4
review comments and readme
HariGS-DB File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,14 @@ | ||
import base64 | ||
import dataclasses | ||
import io | ||
import json | ||
import logging | ||
import os | ||
import pathlib | ||
from pathlib import Path | ||
from unittest.mock import create_autospec | ||
|
||
import yaml | ||
from databricks.labs.blueprint.installation import MockInstallation | ||
from databricks.labs.lsql.backends import MockBackend | ||
from databricks.sdk import WorkspaceClient | ||
|
@@ -15,8 +17,8 @@ | |
from databricks.sdk.service.jobs import BaseJob, BaseRun | ||
from databricks.sdk.service.pipelines import GetPipelineResponse, PipelineStateInfo | ||
from databricks.sdk.service.sql import EndpointConfPair | ||
from databricks.sdk.service.workspace import ExportResponse, GetSecretResponse | ||
|
||
from databricks.sdk.service.workspace import ExportResponse, GetSecretResponse, ObjectInfo | ||
from databricks.sdk.service import iam | ||
from databricks.labs.ucx.hive_metastore.mapping import TableMapping, TableToMigrate | ||
from databricks.labs.ucx.source_code.graph import SourceContainer | ||
from databricks.labs.ucx.source_code.path_lookup import PathLookup | ||
|
@@ -184,11 +186,13 @@ def workspace_client_mock( | |
secret_exists=True, | ||
): | ||
ws = create_autospec(WorkspaceClient) | ||
ws.current_user.me = lambda: iam.User(user_name="[email protected]", groups=[iam.ComplexValue(display="admins")]) | ||
ws.clusters.list.return_value = _id_list(ClusterDetails, cluster_ids) | ||
ws.cluster_policies.list.return_value = _id_list(Policy, policy_ids) | ||
ws.cluster_policies.get = _cluster_policy | ||
ws.pipelines.list_pipelines.return_value = _id_list(PipelineStateInfo, pipeline_ids) | ||
ws.pipelines.get = _pipeline | ||
ws.workspace.get_status = lambda _: ObjectInfo(object_id=123) | ||
ws.jobs.list.return_value = _id_list(BaseJob, job_ids) | ||
ws.jobs.list_runs.return_value = _id_list(BaseRun, jobruns_ids) | ||
ws.warehouses.get_workspace_warehouse_config().data_access_config = _load_list(EndpointConfPair, warehouse_config) | ||
|
@@ -197,6 +201,18 @@ def workspace_client_mock( | |
ws.secrets.get_secret.return_value = GetSecretResponse(key="username", value="SGVsbG8sIFdvcmxkIQ==") | ||
else: | ||
ws.secrets.get_secret = _secret_not_found | ||
download_yaml = yaml.dump( | ||
{ | ||
'version': 1, | ||
'inventory_database': 'ucx_exists', | ||
'connect': { | ||
'host': '...', | ||
'token': '...', | ||
}, | ||
'installed_workspace_ids': [123, 456], | ||
} | ||
) | ||
ws.workspace.download.return_value = io.StringIO(download_yaml) | ||
return ws | ||
|
||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
import io | ||
from unittest.mock import create_autospec | ||
|
||
import yaml | ||
|
||
from databricks.sdk.service.provisioning import Workspace | ||
|
||
from databricks.labs.blueprint.tui import MockPrompts | ||
from databricks.labs.blueprint.wheels import ProductInfo | ||
from databricks.sdk import AccountClient | ||
from databricks.labs.ucx.config import WorkspaceConfig | ||
from databricks.labs.ucx.install import AccountInstaller | ||
from . import workspace_client_mock | ||
|
||
PRODUCT_INFO = ProductInfo.from_class(WorkspaceConfig) | ||
|
||
|
||
def test_join_collection_prompt_no_join(): | ||
account_client = create_autospec(AccountClient) | ||
account_installer = AccountInstaller(account_client) | ||
prompts = MockPrompts( | ||
{ | ||
r".*PRO or SERVERLESS SQL warehouse.*": "1", | ||
r"Open job overview.*": "no", | ||
r"Do you want to join the current.*": "no", | ||
r".*": "", | ||
} | ||
) | ||
account_installer.replace( | ||
prompts=prompts, | ||
product_info=ProductInfo.for_testing(WorkspaceConfig), | ||
) | ||
account_installer.join_collection(123) | ||
account_client.workspaces.list.assert_not_called() | ||
|
||
|
||
def test_join_collection_no_sync_called(): | ||
account_client = create_autospec(AccountClient) | ||
account_installer = AccountInstaller(account_client) | ||
prompts = MockPrompts( | ||
{ | ||
r".*PRO or SERVERLESS SQL warehouse.*": "1", | ||
r"Open job overview.*": "no", | ||
r"Do you want to join the current.*": "yes", | ||
r".*": "", | ||
} | ||
) | ||
account_installer.replace( | ||
prompts=prompts, | ||
product_info=ProductInfo.for_testing(WorkspaceConfig), | ||
) | ||
account_installer.join_collection(123) | ||
account_client.workspaces.list.assert_called() | ||
account_client.get_workspace_client.assert_not_called() | ||
|
||
|
||
def test_join_collection_join_collection_no_installation_id(): | ||
ws = workspace_client_mock() | ||
download_yaml = yaml.dump( | ||
{ | ||
'version': 1, | ||
'inventory_database': 'ucx_exists', | ||
'connect': { | ||
'host': '...', | ||
'token': '...', | ||
}, | ||
} | ||
) | ||
ws.workspace.download.return_value = io.StringIO(download_yaml) | ||
account_client = create_autospec(AccountClient) | ||
account_client.workspaces.list.return_value = [ | ||
Workspace(workspace_id=123, deployment_name="test"), | ||
Workspace(workspace_id=456, deployment_name="test2"), | ||
] | ||
account_client.get_workspace_client.return_value = ws | ||
account_installer = AccountInstaller(account_client) | ||
prompts = MockPrompts( | ||
{ | ||
r".*PRO or SERVERLESS SQL warehouse.*": "1", | ||
r"Open job overview.*": "no", | ||
r"Do you want to join the current.*": "yes", | ||
r"Please select a workspace, the current installation.*": 1, | ||
r".*": "", | ||
} | ||
) | ||
account_installer.replace( | ||
prompts=prompts, | ||
product_info=ProductInfo.for_testing(WorkspaceConfig), | ||
) | ||
|
||
account_installer.join_collection(456) | ||
ws.workspace.upload.assert_called() | ||
assert ws.workspace.upload.call_count == 1 | ||
|
||
|
||
def test_join_collection_join_collection(): | ||
ws = workspace_client_mock() | ||
account_client = create_autospec(AccountClient) | ||
account_client.workspaces.list.return_value = [ | ||
Workspace(workspace_id=123, deployment_name="test"), | ||
Workspace(workspace_id=456, deployment_name="test2"), | ||
] | ||
account_client.get_workspace_client.return_value = ws | ||
account_installer = AccountInstaller(account_client) | ||
prompts = MockPrompts( | ||
{ | ||
r".*PRO or SERVERLESS SQL warehouse.*": "1", | ||
r"Open job overview.*": "no", | ||
r"Do you want to join the current.*": "yes", | ||
r"Please select a workspace, the current installation.*": 1, | ||
r".*": "", | ||
} | ||
) | ||
account_installer.replace( | ||
prompts=prompts, | ||
product_info=ProductInfo.for_testing(WorkspaceConfig), | ||
) | ||
account_installer.join_collection(789) | ||
ws.workspace.upload.assert_called() | ||
assert ws.workspace.upload.call_count == 2 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would this return if user does not have account admin permission?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesnt fail. The first step of failure if a user is not an account admin would be at the workspace.list() cmd