Translate dashboards' paths before their serialized contents are read #3006
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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,22 @@ | ||
| # | ||
| # Acceptance test for deploying dashboards with the following setup: | ||
| # 1. dashboard file is outside the bundle root | ||
| # 2. sync root is one level above bundle root | ||
| # | ||
| bundle: | ||
| name: deploy-dashboard-outside-bundle-root-$UNIQUE_NAME | ||
|
There was a problem hiding this comment. what's the difference between this test and simple_syncroot? The config seems to be very similar? Can you add a comment here on in the script explaining the purpose of this test and difference from simple_syncroot? There was a problem hiding this comment. added comments in 81972c4 |
||
|
|
||
| sync: | ||
| paths: | ||
| - .. | ||
| include: | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - .. | ||
|
|
||
| resources: | ||
| dashboards: | ||
| dashboard1: | ||
| display_name: $DASHBOARD_DISPLAY_NAME | ||
| warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID | ||
| embed_credentials: true | ||
|
There was a problem hiding this comment. (optional followup) If you are setting There was a problem hiding this comment. I suggest we do it in a follow-up PR. this PR does not make use of testserver yet |
||
| file_path: ../sample-dashboard.lvdash.json | ||
| parent_path: /Users/$CURRENT_USER_NAME | ||
This file contains hidden or 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,23 @@ | ||
|
|
||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-outside-bundle-root-[UNIQUE_NAME]/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! | ||
|
|
||
| >>> [CLI] lakeview get [DASHBOARD_ID] | ||
| { | ||
| "lifecycle_state": "ACTIVE", | ||
| "parent_path": "/Users/[USERNAME]", | ||
| "path": "/Users/[USERNAME]/test bundle-deploy-dashboard [UUID].lvdash.json", | ||
| "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}" | ||
| } | ||
|
|
||
| >>> [CLI] bundle destroy --auto-approve | ||
| The following resources will be deleted: | ||
| delete dashboard dashboard1 | ||
|
|
||
| All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-outside-bundle-root-[UNIQUE_NAME]/default | ||
|
|
||
| Deleting files... | ||
| Destroy complete! |
This file contains hidden or 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,18 @@ | ||
| DASHBOARD_DISPLAY_NAME="test bundle-deploy-dashboard $(uuid)" | ||
| if [ -z "$CLOUD_ENV" ]; then | ||
| DASHBOARD_DISPLAY_NAME="test bundle/deploy/ 6260d50f-e8ff-4905-8f28-812345678903" # use hard-coded uuid when running locally | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| export TEST_DEFAULT_WAREHOUSE_ID="warehouse-1234" | ||
| fi | ||
| cp $TESTDIR/../simple/sample-dashboard.lvdash.json ../. | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| export DASHBOARD_DISPLAY_NAME | ||
| envsubst < databricks.yml.tmpl > databricks.yml | ||
|
|
||
| cleanup() { | ||
| trace $CLI bundle destroy --auto-approve | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| trace $CLI bundle deploy | ||
| DASHBOARD_ID=$($CLI bundle summary --output json | jq -r '.resources.dashboards.dashboard1.id') | ||
| trace $CLI lakeview get $DASHBOARD_ID | jq '{lifecycle_state, parent_path, path, serialized_dashboard}' | ||
This file contains hidden or 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,44 @@ | ||
| Local = true | ||
| Cloud = true | ||
| RequiresWarehouse = true | ||
|
|
||
| Ignore = [ | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "databricks.yml", | ||
| "sample-dashboard.lvdash.json", | ||
| ] | ||
|
|
||
| [[Repls]] | ||
| Old = "[0-9a-f]{32}" | ||
| New = "[DASHBOARD_ID]" | ||
|
|
||
| [[Repls]] | ||
| # Windows: | ||
| Old = 'The system cannot find the file specified.' | ||
| New = 'no such file or directory' | ||
|
|
||
| [[Server]] | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Pattern = "POST /api/2.0/lakeview/dashboards" | ||
| Response.Body = ''' | ||
| { | ||
| "dashboard_id":"1234567890abcdef1234567890abcdef" | ||
| } | ||
| ''' | ||
|
|
||
| [[Server]] | ||
| Pattern = "POST /api/2.0/lakeview/dashboards/{dashboard_id}/published" | ||
anton-107 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| [[Server]] | ||
| Pattern = "GET /api/2.0/lakeview/dashboards/{dashboard_id}" | ||
| Response.Body = ''' | ||
| { | ||
| "dashboard_id":"1234567890abcdef1234567890abcdef", | ||
| "display_name": "test dashboard 6260d50f-e8ff-4905-8f28-812345678903", | ||
| "lifecycle_state": "ACTIVE", | ||
| "path": "/Users/[USERNAME]/test bundle-deploy-dashboard 6260d50f-e8ff-4905-8f28-812345678903.lvdash.json", | ||
| "parent_path": "/Users/[email protected]", | ||
| "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}" | ||
| } | ||
| ''' | ||
|
|
||
| [[Server]] | ||
| Pattern = "DELETE /api/2.0/lakeview/dashboards/{dashboard_id}" | ||
This file contains hidden or 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 hidden or 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,9 +1,23 @@ | ||
|
|
||
| >>> [CLI] bundle deploy | ||
| Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-test-[UNIQUE_NAME]/default/files... | ||
| Deploying resources... | ||
| Updating deployment state... | ||
| Deployment complete! | ||
|
|
||
| >>> [CLI] lakeview get [DASHBOARD_ID] | ||
| { | ||
| "lifecycle_state": "ACTIVE", | ||
| "parent_path": "/Users/[USERNAME]", | ||
| "path": "/Users/[USERNAME]/test bundle-deploy-dashboard [UUID].lvdash.json", | ||
| "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}" | ||
| } | ||
|
|
||
| >>> [CLI] bundle destroy --auto-approve | ||
| The following resources will be deleted: | ||
| delete dashboard dashboard1 | ||
|
|
||
| All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-dashboard-test-[UNIQUE_NAME]/default | ||
|
|
||
| Deleting files... | ||
| Destroy complete! |
This file contains hidden or 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,4 +1,3 @@ | ||
| Local = true | ||
| Cloud = true | ||
| RequiresWarehouse = true | ||
|
|
||
This file contains hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -47,13 +47,22 @@ func (err ErrIsNotNotebook) Error() string { | |
| return fmt.Sprintf("file at %s is not a notebook", err.path) | ||
| } | ||
|
|
||
| type translatePaths struct { | ||
| dashboardsOnly bool | ||
| } | ||
|
|
||
| // TranslatePaths converts paths to local notebook files into paths in the workspace file system. | ||
| func TranslatePaths() bundle.Mutator { | ||
| return &translatePaths{} | ||
| } | ||
|
|
||
| // TranslatePathsDashboards converts paths to local dashboard files into paths in the workspace file system. | ||
| func TranslatePathsDashboards() bundle.Mutator { | ||
| return &translatePaths{ | ||
| dashboardsOnly: true, | ||
|
There was a problem hiding this comment. Given that you already have a separate mutator, why not just implement Apply for it directly and keep translatePaths option-less. There was a problem hiding this comment. I tried that, but i do not want to copy translation logic (structs and methods) nor i want to make it public to other packages, so i decided this is the "cleanest" way to get this done There was a problem hiding this comment. Both mutators can live in the same package and in the same file. You just need to add new struct and Apply: There was a problem hiding this comment. please review #3031 to see if you like it better There was a problem hiding this comment. Hrm, I expected it to be simpler than that. We can proceed with either, up to you. There was a problem hiding this comment. I suggest we merge this first, then I will rebase the refactor branch and evaluate which version i like better |
||
| } | ||
| } | ||
|
|
||
| func (m *translatePaths) Name() string { | ||
| return "TranslatePaths" | ||
| } | ||
|
|
@@ -297,13 +306,21 @@ func (m *translatePaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn | |
|
|
||
| err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { | ||
| var err error | ||
|
|
||
| translations := []func(context.Context, dyn.Value) (dyn.Value, error){ | ||
| t.applyJobTranslations, | ||
| t.applyPipelineTranslations, | ||
| t.applyArtifactTranslations, | ||
| t.applyAppsTranslations, | ||
| } | ||
|
|
||
| if m.dashboardsOnly { | ||
| translations = []func(context.Context, dyn.Value) (dyn.Value, error){ | ||
| t.applyDashboardTranslations, | ||
| } | ||
| } | ||
|
|
||
| for _, fn := range translations { | ||
| v, err = fn(ctx, v) | ||
| if err != nil { | ||
| return dyn.InvalidValue, err | ||
|
|
||
Oops, something went wrong.
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.
Nit: looks like a separate directory from the directory with the
.ymlfile.