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

Add support for dashboards in deployment bind/unbind commands #2516

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

anton-107
Copy link
Contributor

@anton-107 anton-107 commented Mar 18, 2025

Changes

  1. Changed FindResourceByConfigKey to return dashboard resources
  2. Changed deploy phase to detect dashboard recreation and request user's approval before deploying

Why

This PR adds support for dashboard resources in deployment operations, enabling users to:

  • Bind experiments using databricks bundle deployment bind <mydashboard_key> <mydashboard_id>
  • Unbind experiments using databricks bundle deployment unbind <mydashboard_key>

Where:

  • mydashboard_key is a resource key defined in the bundle's .yml file
  • mydashboard_id references an existing dashboard in the Databricks workspace

These capabilities allow for more flexible resource management of dashboards within bundles.

Tests

Added two new acceptance tests that tests bind and unbind methods together with bundle deployment and destruction, including a case of dashboard recreation

@anton-107 anton-107 temporarily deployed to test-trigger-is March 18, 2025 15:28 — with GitHub Actions Inactive
@anton-107 anton-107 marked this pull request as ready for review March 18, 2025 16:05
@anton-107 anton-107 temporarily deployed to test-trigger-is March 19, 2025 13:15 — with GitHub Actions Inactive
@anton-107 anton-107 force-pushed the anton-107/bind-dashboards branch from f77bc94 to 6034d67 Compare March 20, 2025 09:31
@anton-107 anton-107 temporarily deployed to test-trigger-is March 20, 2025 09:31 — with GitHub Actions Inactive
DASHBOARD_DISPLAY_NAME="test dashboard $(uuid)"
if [ -z "$CLOUD_ENV" ]; then
DASHBOARD_DISPLAY_NAME="test dashboard 6260d50f-e8ff-4905-8f28-812345678903" # use hard-coded uuid when running locally
export TEST_DEFAULT_WAREHOUSE_ID="warehouse-1234"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any downside for hardcoding these in acceptance_test.go if CLOUD_ENV is "" and they are not set already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the only thing i can think of - this style keeps this local to the test and does not encourage variables that are too use-case specific to be used in unrelated tests

Copy link
Contributor

@shreyas-goenka shreyas-goenka left a comment

Choose a reason for hiding this comment

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

Can we fix Dashboard recreation on bind before we proceed with this? I would consider recreation of dashboards on bind + subsequent deployment to be a correctness issue.

@anton-107
Copy link
Contributor Author

Can we fix Dashboard recreation on bind before we proceed with this? I would consider recreation of dashboards on bind + subsequent deployment to be a correctness issue.

We are not able to fix the Dashboard re-creation, as there is not API to move a dashboard to a different location without changing its ID. Instead, we will introduce a warning check for end users that a Dashboard would be re-created on deployment

@anton-107 anton-107 force-pushed the anton-107/bind-dashboards branch from 6034d67 to 7445444 Compare March 21, 2025 12:41
@anton-107 anton-107 temporarily deployed to test-trigger-is March 21, 2025 12:41 — with GitHub Actions Inactive
@anton-107 anton-107 temporarily deployed to test-trigger-is March 21, 2025 12:44 — with GitHub Actions Inactive
@anton-107 anton-107 temporarily deployed to test-trigger-is March 21, 2025 16:05 — with GitHub Actions Inactive
@anton-107 anton-107 added this pull request to the merge queue Mar 24, 2025
Merged via the queue into main with commit e8c6639 Mar 24, 2025
9 checks passed
@anton-107 anton-107 deleted the anton-107/bind-dashboards branch March 24, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants