Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 214
Add error for when an etag is specified in dashboard configuration#3723
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
83a8d5bf6fe7dbd41124aeb9c2b37ea6f0c781a2f4302114372402c84a0b366File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| resources: | ||
| dashboards: | ||
| foobar: | ||
| display_name: foobar | ||
| etag: "1234567890" | ||
| serialized_dashboard: "{}" |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| >>> [CLI] bundle validate | ||
| Error: dashboard "foobar" has an etag set. Etags must not be set in bundle configuration | ||
| at resources.dashboards.foobar | ||
| in databricks.yml:6:7 | ||
| Name: test-bundle | ||
| Target: default | ||
| Workspace: | ||
| User: [USERNAME] | ||
| Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default | ||
| Found 1 error | ||
| Exit code: 1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| trace $CLI bundle validate |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package validate | ||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/libs/diag" | ||
| "github.com/databricks/cli/libs/dyn" | ||
| ) | ||
| func ValidateDashboardEtags() bundle.ReadOnlyMutator { | ||
| return &validateDashboardEtags{} | ||
| } | ||
| type validateDashboardEtags struct{ bundle.RO } | ||
| func (v *validateDashboardEtags) Name() string { | ||
| return "validate:validate_dashboard_etags" | ||
| } | ||
| func (v *validateDashboardEtags) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { | ||
| // No dashboards should have etags set. They are purely internal state. | ||
| for k, dashboard := range b.Config.Resources.Dashboards { | ||
| if dashboard.Etag != "" { | ||
| return diag.Diagnostics{ | ||
| { | ||
| Severity: diag.Error, | ||
| Summary: fmt.Sprintf("dashboard %q has an etag set. Etags must not be set in bundle configuration", dashboard.DisplayName), | ||
| Paths: []dyn.Path{dyn.MustPathFromString("resources.dashboards." + k)}, | ||
| Locations: b.Config.GetLocations("resources.dashboards." + k), | ||
| }, | ||
| } | ||
| } | ||
| } | ||
| return nil | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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.
Would not resourcemutator be a better place for this? Currently this misses resources set by PyDABs.
Related, can we have an abstraction similar to SetDefault() that allows us easily adding output-only fields?
MustNotBeSet("resources.dashboards.*.etag"),
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.
This validation happens after PyDABs resources are loaded.
Longer term, perhaps. We can autogenerate those based on OUTPUT only or other annotations. For now we don't need to generalize.
Uh oh!
There was an error while loading. Please reload this page.
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.
In this case I opted for the static types because those are easier to discover / navigate. Eventually we can use the dynamic paths but I don't want to do that prematurely here.
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.
Right, I did not notice.