-
Notifications
You must be signed in to change notification settings - Fork 139
direct: Generate resources config based on OpenAPI field behaviours #4391
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
base: main
Are you sure you want to change the base?
Changes from all commits
77f53e4
4f696c1
b600902
d110081
2d8ae46
d66752e
e0304c7
6f59b1a
c1a4be0
f29a7b3
fb8bda9
13e4d1a
1cfa05a
006118c
0a647f2
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 |
|---|---|---|
|
|
@@ -54,3 +54,6 @@ dist/ | |
| # Go workspace file | ||
| go.work | ||
| go.work.sum | ||
|
|
||
| # Fetched based on recorded hash | ||
| .codegen/openapi.json | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,6 +170,21 @@ generate: | |
| @echo "Generating CLI code..." | ||
| $(GENKIT_BINARY) update-sdk | ||
|
|
||
| .codegen/openapi.json: .codegen/_openapi_sha | ||
| wget -O $@.tmp "https://openapi.dev.databricks.com/$$(cat $<)/specs/all-internal.json" && mv $@.tmp $@ && touch $@ | ||
|
|
||
| generate-direct: generate-direct-apitypes generate-direct-resources | ||
| generate-direct-apitypes: bundle/direct/dresources/apitypes.generated.yml | ||
| generate-direct-resources: bundle/direct/dresources/resources.generated.yml | ||
| generate-direct-clean: | ||
| rm -f bundle/direct/dresources/apitypes.generated.yml bundle/direct/dresources/resources.generated.yml | ||
| .PHONY: generate-direct generate-direct-apitypes generate-direct-resources generate-direct-clean | ||
|
Contributor
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. Isn't this cleared by
Contributor
Author
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. no, it's additive |
||
|
|
||
| bundle/direct/dresources/apitypes.generated.yml: ./bundle/direct/tools/generate_apitypes.py .codegen/openapi.json acceptance/bundle/refschema/out.fields.txt | ||
| python3 $^ > $@ | ||
|
|
||
| bundle/direct/dresources/resources.generated.yml: ./bundle/direct/tools/generate_resources.py .codegen/openapi.json bundle/direct/dresources/apitypes.generated.yml acceptance/bundle/refschema/out.fields.txt | ||
| python3 $^ > $@ | ||
|
|
||
| .PHONY: lint lintfull tidy lintcheck fmt fmtfull test test-unit test-acc test-slow test-slow-unit test-slow-acc cover showcover build snapshot snapshot-release schema integration integration-short acc-cover acc-showcover docs ws wsfix links checks test-update test-update-templates generate-out-test-toml test-update-aws test-update-all generate-validation | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -360,6 +360,7 @@ func prepareChanges(ctx context.Context, adapter *dresources.Adapter, localDiff, | |
|
|
||
| func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, changes deployplan.Changes, remoteState any) error { | ||
| cfg := adapter.ResourceConfig() | ||
| generatedCfg := adapter.GeneratedResourceConfig() | ||
|
Contributor
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. Not clear what is
Contributor
Author
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. what names would you suggest? |
||
|
|
||
| for pathString, ch := range changes { | ||
| path, err := structpath.Parse(pathString) | ||
|
|
@@ -385,6 +386,9 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change | |
| } else if shouldSkip(cfg, path, ch) { | ||
| ch.Action = deployplan.Skip | ||
| ch.Reason = deployplan.ReasonBuiltinRule | ||
| } else if shouldSkip(generatedCfg, path, ch) { | ||
| ch.Action = deployplan.Skip | ||
| ch.Reason = deployplan.ReasonAPISchema | ||
| } else if ch.New == nil && ch.Old == nil && ch.Remote != nil && path.IsDotString() { | ||
| // The field was not set by us, but comes from the remote state. | ||
| // This could either be server-side default or a policy. | ||
|
|
@@ -395,6 +399,9 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change | |
| } else if action := shouldUpdateOrRecreate(cfg, path); action != deployplan.Undefined { | ||
| ch.Action = action | ||
| ch.Reason = deployplan.ReasonBuiltinRule | ||
| } else if action := shouldUpdateOrRecreate(generatedCfg, path); action != deployplan.Undefined { | ||
| ch.Action = action | ||
| ch.Reason = deployplan.ReasonAPISchema | ||
| } else { | ||
| ch.Action = deployplan.Update | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,8 +94,9 @@ type Adapter struct { | |
| overrideChangeDesc *calladapt.BoundCaller | ||
| doResize *calladapt.BoundCaller | ||
|
|
||
| resourceConfig *ResourceLifecycleConfig | ||
| keyedSlices map[string]any | ||
| resourceConfig *ResourceLifecycleConfig | ||
| generatedResourceConfig *ResourceLifecycleConfig | ||
| keyedSlices map[string]any | ||
| } | ||
|
|
||
| func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceClient) (*Adapter, error) { | ||
|
|
@@ -112,19 +113,20 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC | |
| } | ||
| impl := outs[0] | ||
| adapter := &Adapter{ | ||
| prepareState: nil, | ||
| remapState: nil, | ||
| doRefresh: nil, | ||
| doDelete: nil, | ||
| doCreate: nil, | ||
| doUpdate: nil, | ||
| doUpdateWithID: nil, | ||
| doResize: nil, | ||
| waitAfterCreate: nil, | ||
| waitAfterUpdate: nil, | ||
| overrideChangeDesc: nil, | ||
| resourceConfig: GetResourceConfig(resourceType), | ||
| keyedSlices: nil, | ||
| prepareState: nil, | ||
| remapState: nil, | ||
| doRefresh: nil, | ||
| doDelete: nil, | ||
| doCreate: nil, | ||
| doUpdate: nil, | ||
| doUpdateWithID: nil, | ||
| doResize: nil, | ||
| waitAfterCreate: nil, | ||
| waitAfterUpdate: nil, | ||
| overrideChangeDesc: nil, | ||
| resourceConfig: GetResourceConfig(resourceType), | ||
| generatedResourceConfig: GetGeneratedResourceConfig(resourceType), | ||
| keyedSlices: nil, | ||
| } | ||
|
|
||
| err = adapter.initMethods(impl) | ||
|
|
@@ -355,6 +357,10 @@ func (a *Adapter) ResourceConfig() *ResourceLifecycleConfig { | |
| return a.resourceConfig | ||
| } | ||
|
|
||
| func (a *Adapter) GeneratedResourceConfig() *ResourceLifecycleConfig { | ||
|
Contributor
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. nit:
Contributor
Author
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. OpenAPI is just a medium, could be protobuf in the future. Important part that it comes from the spec. |
||
| return a.generatedResourceConfig | ||
| } | ||
|
|
||
| func (a *Adapter) IsFieldInRecreateOnChanges(path *structpath.PathNode) bool { | ||
| for _, p := range a.resourceConfig.RecreateOnChanges { | ||
| if path.HasPrefix(p) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Generated, do not edit. Override via apitypes.yml | ||
|
|
||
| alerts: sql.AlertV2 | ||
|
|
||
| apps: apps.App | ||
|
|
||
| clusters: compute.ClusterSpec | ||
|
|
||
| dashboards: dashboards.Dashboard | ||
|
|
||
| database_catalogs: database.DatabaseCatalog | ||
|
|
||
| database_instances: database.DatabaseInstance | ||
|
|
||
| experiments: ml.CreateExperiment | ||
|
|
||
| jobs: jobs.JobSettings | ||
|
|
||
| model_serving_endpoints: serving.CreateServingEndpoint | ||
|
|
||
| models: ml.CreateModelRequest | ||
|
|
||
| pipelines: pipelines.CreatePipeline | ||
|
|
||
| quality_monitors: catalog.CreateMonitor | ||
|
|
||
| registered_models: catalog.RegisteredModelInfo | ||
|
|
||
| schemas: catalog.CreateSchema | ||
|
|
||
| secret_scopes: workspace.CreateScope | ||
|
|
||
| sql_warehouses: sql.EditWarehouseRequest | ||
|
|
||
| synced_database_tables: database.SyncedDatabaseTable | ||
|
|
||
| volumes: catalog.CreateVolumeRequestContent |
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.
We commit these files anyways. Why remove them / have this makefile rule?
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.
Helps during development of those scripts.