8000 fix: preserve names on idempotent updates by eddiethedean · Pull Request #74 · dmux/minilake · GitHub
[go: up one dir, main page]
Skip to content

fix: preserve names on idempotent updates #74

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.

Sign up for GitHub

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

Merged
dmux merged 4 commits into from
Oct 29, 2026
Merged

fix: preserve names on idempotent updates #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd1d1dd
fix: preserve names on idempotent updates
eddiethedean Oct 26, 2026
f003324
style: apply Ruff formatting to resolver changes
eddiethedean Oct 26, 2026
862b4c3
test: cover idempotent auto-resolved updates
eddiethedean Oct 26, 2026
0f99ee7
Merge branch 'main' into codex/fix-issue-53-auto-resolve
dmux Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/minilake/services/alerts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ def _display_name_taken(display_name: Optional[str], exclude_id: Optional[str] =
)


def _resolve_display_name(display_name: Optional[str]) -> str:
def _resolve_display_name(display_name: Optional[str], exclude_id: Optional[str] = None) -> str:
"""Append ` (n)` until the name is free, mirroring `auto_resolve_display_name`."""
base = display_name or "New alert"
if not _display_name_taken(base):
if not _display_name_taken(base, exclude_id=exclude_id):
return base
suffix = 1
while _display_name_taken(f"{base} ({suffix})"):
while _display_name_taken(f"{base} ({suffix})", exclude_id=exclude_id):
suffix += 1
return f"{base} ({suffix})"

Expand Down Expand Up @@ -362,7 +362,7 @@ async def update_alert(
display_name = updates.get("display_name")
if display_name is not None:
if req.auto_resolve_display_name:
updates["display_name"] = _resolve_display_name(display_name)
updates["display_name"] = _resolve_display_name(display_name, exclude_id=alert_id)
elif _display_name_taken(display_name, exclude_id=alert_id):
raise DatabricksError(
error_code="RESOURCE_ALREADY_EXISTS",
Expand Down
8 changes: 4 additions & 4 deletions src/minilake/services/saved_queries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ def _display_name_taken(display_name: Optional[str], exclude_id: Optional[str] =
)


def _resolve_display_name(display_name: Optional[str]) -> str:
def _resolve_display_name(display_name: Optional[str], exclude_id: Optional[str] = None) -> str:
"""Append ` (n)` until the name is free, mirroring `auto_resolve_display_name`."""
base = display_name or "New query"
if not _display_name_taken(base):
if not _display_name_taken(base, exclude_id=exclude_id):
return base
suffix = 1
while _display_name_taken(f"{base} ({suffix})"):
while _display_name_taken(f"{base} ({suffix})", exclude_id=exclude_id):
suffix += 1
return f"{base} ({suffix})"

Expand Down Expand Up @@ -234,7 +234,7 @@ async def update_query(
display_name = updates.get("display_name")
if display_name is not None:
if req.auto_resolve_display_name:
updates["display_name"] = _resolve_display_name(display_name)
updates["display_name"] = _resolve_display_name(display_name, exclude_id=query_id)
elif _display_name_taken(display_name, exclude_id=query_id):
raise DatabricksError(
error_code="RESOURCE_ALREADY_EXISTS",
Expand Down
29 changes: 29 additions & 0 deletions tests/test_alerts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,35 @@ def test_update_alert_honours_update_mask(workspace_client: WorkspaceClient, cou
assert updated.custom_subject == "keep me"


@pytest.mark.crud
def test_auto_resolve_display_name_on_update_is_idempotent(workspace_client: WorkspaceClient, counting_query: str):
"""Updates preserve their name and suffix only for collisions with other alerts."""
alert = workspace_client.alerts.create(
alert=CreateAlertRequestAlert(display_name="unchanged", query_id=counting_query)
)

for _ in range(2):
updated = workspace_client.alerts.update(
id=alert.id,
update_mask="display_name",
auto_resolve_display_name=True,
alert=UpdateAlertRequestAlert(display_name="unchanged"),
)
assert updated.display_name == "unchanged"

workspace_client.alerts.create(alert=CreateAlertRequestAlert(display_name="occupied", query_id=counting_query))
workspace_client.alerts.create(alert=CreateAlertRequestAlert(display_name="occupied (1)", query_id=counting_query))

for _ in range(2):
updated = workspace_client.alerts.update(
id=alert.id,
update_mask="display_name",
auto_resolve_display_name=True,
alert=UpdateAlertRequestAlert(display_name="occupied"),
)
assert updated.display_name == "occupied (2)"


@pytest.mark.crud
def test_delete_alert(workspace_client: WorkspaceClient, counting_query: str):
created = workspace_client.alerts.create(
Expand Down
29 changes: 29 additions & 0 deletions tests/test_saved_queries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,35 @@ def test_update_multiple_masked_fields(workspace_client: WorkspaceClient):
assert updated.query_text == "SELECT 42"


@pytest.mark.crud
def test_auto_resolve_display_name_on_update_is_idempotent(workspace_client: WorkspaceClient):
"""Updates preserve their name and suffix only for collisions with other queries."""
query = workspace_client.queries.create(
query=CreateQueryRequestQuery(display_name="unchanged", query_text="SELECT 1")
)

for _ in range(2):
updated = workspace_client.queries.update(
id=query.id,
update_mask="display_name",
auto_resolve_display_name=True,
query=UpdateQueryRequestQuery(display_name="unchanged"),
)
assert updated.display_name == "unchanged"

workspace_client.queries.create(query=CreateQueryRequestQuery(display_name="occupied", query_text="SELECT 2"))
workspace_client.queries.create(query=CreateQueryRequestQuery(display_name="occupied (1)", query_text="SELECT 3"))

for _ in range(2):
updated = workspace_client.queries.update(
id=query.id,
update_mask="display_name",
auto_resolve_display_name=True,
query=UpdateQueryRequestQuery(display_name="occupied"),
)
assert updated.display_name == "occupied (2)"


@pytest.mark.crud
def test_delete_saved_query(workspace_client: WorkspaceClient):
"""A deleted query is gone from both get and list."""
Expand Down
Loading
0