-
Notifications
You must be signed in to change notification settings - Fork 80
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
More ownership classes #2999
More ownership classes #2999
Changes from all commits
18acdc0
b7b0bb2
d0a6f6d
3a411c6
5b15124
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,8 @@ | |
from databricks.labs.ucx.assessment.clusters import ( | ||
ClustersCrawler, | ||
PoliciesCrawler, | ||
ClusterOwnership, | ||
ClusterDetailsOwnership, | ||
ClusterInfoOwnership, | ||
ClusterPolicyOwnership, | ||
) | ||
|
||
|
@@ -76,9 +77,12 @@ def test_cluster_ownership(ws, runtime_ctx, make_cluster, make_user, inventory_s | |
|
||
# Verify ownership is as expected. | ||
administrator_locator = runtime_ctx.administrator_locator | ||
ownership = ClusterOwnership(administrator_locator) | ||
assert ownership.owner_of(my_cluster_record) == ws.current_user.me().user_name | ||
assert ownership.owner_of(their_cluster_record) == another_user.user_name | ||
info_ownership = ClusterInfoOwnership(administrator_locator) | ||
assert info_ownership.owner_of(my_cluster_record) == ws.current_user.me().user_name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
assert info_ownership.owner_of(their_cluster_record) == another_user.user_name | ||
details_ownership = ClusterDetailsOwnership(administrator_locator) | ||
assert details_ownership.owner_of(ws.clusters.get(my_cluster.cluster_id)) == ws.current_user.me().user_name | ||
assert details_ownership.owner_of(ws.clusters.get(their_cluster.cluster_id)) == another_user.user_name | ||
|
||
|
||
def test_cluster_crawler_mlr_no_isolation(ws, make_cluster, inventory_schema, sql_backend): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,11 @@ | |
from databricks.labs.ucx.assessment.azure import AzureServicePrincipalCrawler | ||
from databricks.labs.ucx.assessment.clusters import ( | ||
ClustersCrawler, | ||
PoliciesCrawler, | ||
ClusterOwnership, | ||
ClusterDetailsOwnership, | ||
ClusterInfoOwnership, | ||
ClusterInfo, | ||
ClusterPolicyOwnership, | ||
PoliciesCrawler, | ||
PolicyInfo, | ||
) | ||
from databricks.labs.ucx.framework.crawlers import SqlBackend | ||
|
@@ -185,27 +186,48 @@ def test_unsupported_clusters(): | |
assert result_set[0].failures == '["cluster type not supported : LEGACY_PASSTHROUGH"]' | ||
|
||
|
||
def test_cluster_owner_creator() -> None: | ||
def test_cluster_info_owner_creator() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
|
||
ownership = ClusterOwnership(admin_locator) | ||
ownership = ClusterInfoOwnership(admin_locator) | ||
owner = ownership.owner_of(ClusterInfo(creator="bob", cluster_id="1", success=1, failures="[]")) | ||
|
||
assert owner == "bob" | ||
admin_locator.get_workspace_administrator.assert_not_called() | ||
|
||
|
||
def test_cluster_owner_creator_unknown() -> None: | ||
def test_cluster_info_owner_creator_unknown() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
admin_locator.get_workspace_administrator.return_value = "an_admin" | ||
|
||
ownership = ClusterOwnership(admin_locator) | ||
ownership = ClusterInfoOwnership(admin_locator) | ||
owner = ownership.owner_of(ClusterInfo(creator=None, cluster_id="1", success=1, failures="[]")) | ||
|
||
assert owner == "an_admin" | ||
admin_locator.get_workspace_administrator.assert_called_once() | ||
|
||
|
||
def test_cluster_details_owner_creator() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
|
||
ownership = ClusterDetailsOwnership(admin_locator) | ||
owner = ownership.owner_of(ClusterDetails(creator_user_name="bob", cluster_id="1")) | ||
|
||
assert owner == "bob" | ||
admin_locator.get_workspace_administrator.assert_not_called() | ||
|
||
|
||
def test_cluster_details_owner_creator_unknown() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
admin_locator.get_workspace_administrator.return_value = "an_admin" | ||
|
||
ownership = ClusterDetailsOwnership(admin_locator) | ||
owner = ownership.owner_of(ClusterDetails(cluster_id="1")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the default for |
||
|
||
assert owner == "an_admin" | ||
admin_locator.get_workspace_administrator.assert_called_once() | ||
|
||
|
||
def test_policy_crawler(): | ||
ws = mock_workspace_client( | ||
policy_ids=['single-user-with-spn', 'single-user-with-spn-policyid', 'single-user-with-spn-no-sparkversion'], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ | |
|
||
import pytest | ||
from databricks.labs.lsql.backends import MockBackend | ||
from databricks.sdk.service.jobs import BaseJob, JobSettings | ||
from databricks.sdk.service.jobs import BaseJob, JobSettings, Job | ||
|
||
from databricks.labs.ucx.assessment.jobs import JobInfo, JobOwnership, JobsCrawler, SubmitRunsCrawler | ||
from databricks.labs.ucx.assessment.jobs import JobInfo, JobInfoOwnership, JobsCrawler, SubmitRunsCrawler, JobOwnership | ||
from databricks.labs.ucx.framework.owners import AdministratorLocator | ||
|
||
from .. import mock_workspace_client | ||
|
@@ -135,22 +135,43 @@ def test_job_run_crawler(jobruns_ids, cluster_ids, run_ids, failures): | |
assert result[0].failures == failures | ||
|
||
|
||
def test_pipeline_owner_creator() -> None: | ||
def test_jobinfo_owner_creator() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
|
||
ownership = JobOwnership(admin_locator) | ||
ownership = JobInfoOwnership(admin_locator) | ||
owner = ownership.owner_of(JobInfo(creator="bob", job_id="1", success=1, failures="[]")) | ||
|
||
assert owner == "bob" | ||
admin_locator.get_workspace_administrator.assert_not_called() | ||
|
||
|
||
def test_pipeline_owner_creator_unknown() -> None: | ||
def test_jobinfo_owner_creator_unknown() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
admin_locator.get_workspace_administrator.return_value = "an_admin" | ||
|
||
ownership = JobOwnership(admin_locator) | ||
ownership = JobInfoOwnership(admin_locator) | ||
owner = ownership.owner_of(JobInfo(creator=None, job_id="1", success=1, failures="[]")) | ||
|
||
assert owner == "an_admin" | ||
admin_locator.get_workspace_administrator.assert_called_once() | ||
|
||
|
||
def test_job_owner_creator() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
|
||
ownership = JobOwnership(admin_locator) | ||
owner = ownership.owner_of(Job(creator_user_name="bob", job_id=1)) | ||
|
||
assert owner == "bob" | ||
admin_locator.get_workspace_administrator.assert_not_called() | ||
|
||
|
||
def test_job_owner_creator_unknown() -> None: | ||
admin_locator = create_autospec(AdministratorLocator) | ||
admin_locator.get_workspace_administrator.return_value = "an_admin" | ||
|
||
ownership = JobOwnership(admin_locator) | ||
owner = ownership.owner_of(Job(job_id=1)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise here with |
||
|
||
assert owner == "an_admin" | ||
admin_locator.get_workspace_administrator.assert_called_once() |
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.
where is it coming from? we don't have a concept of
cluster details
in our object model - we just have a cluster.