Add bazel generation of alloydb resources#16569
Add bazel generation of alloydb resources#16569SirGitsalot merged 2 commits intoGoogleCloudPlatform:mainfrom
Conversation
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
melinath
left a comment
melinath
left a comment
There was a problem hiding this comment.
Overall this looks good; I think taking the approach of mmv1 as library vs downstreams makes sense 👍
When I try to run bazel locally I get the error:
Error in read: java.io.FileNotFoundException: /Users/{{username}}/projects/magic-modules/go.work (No such file or directory)
I'm not blocking on it because it may be a misconfiguration on my part and this work is still in early phases, but wanted to flag it in case.
attn @zli82016 @c2thorn FYI since this will be a significant shift in how TPG / TGC generation works in the future.
attn @ScottSuarez since you were interested in this work.
| @@ -241,6 +268,20 @@ func (t *Terraform) GenerateResourceSweeper(object api.Resource, templateData Te | |||
| templateData.GenerateSweeperFile(targetFilePath, object) | |||
| } | |||
|
|
|||
| // GenerateResourceMetadataFile is used by the Bazel version of the MM compiler to generate the sweeper for | |||
| // the specified resource. It panics if the resource does not use a sweeper. | |||
There was a problem hiding this comment.
I assume the panic is going to change in the future, to support resources without sweepers?
There was a problem hiding this comment.
Resources without sweepers are supported now, the compiler won't be called if the mm_resource does not include has_sweeper = True. This is catching the case where the rule is configured with has_sweeper = True but the resource.yaml file is configured with exclude_sweeper: true.
| @@ -163,6 +165,20 @@ func (t *Terraform) GenerateResource(object api.Resource, templateData TemplateD | |||
| } | |||
| } | |||
|
|
|||
| // GenerateResourceFile is the Bazel counterpart to GenerateResource(), generating *only() the .go file and | |||
| // taking the full path to the output file to generate rather than implicitly generating the path. | |||
There was a problem hiding this comment.
not a blocker, might be what you're planning to do next, but what would you think about making GenerateResource and friends take a full path so we can merge these implementations?
There was a problem hiding this comment.
I'll take a look at doing it. Despite the all of the Generate* methods being exported from the package, the only one that appears to be called from outside the package is the all encompassing Generate() so it should be doable.
Yeah, that's expected. You need a |
c2a9384
| var product api.Product | ||
| api.Compile(*productFlag, &product) | ||
| if *productOverrideFlag != "" { | ||
| var override api.Product | ||
| api.Compile(*productOverrideFlag, &override) | ||
| api.Merge(reflect.ValueOf(product), reflect.ValueOf(override), *versionFlag) | ||
| } | ||
| if !product.ExistsAtVersionOrLower(*versionFlag) { | ||
| log.Fatalf("product %q does not support version %q", *productNameFlag, *versionFlag) | ||
| } | ||
| product.Version = product.VersionObjOrClosest(*versionFlag) | ||
|
|
||
| if *resourceFlag != "" { | ||
| var resource api.Resource | ||
| api.Compile(*resourceFlag, &resource) | ||
| if *resourceOverrideFlag != "" { | ||
| var override api.Resource | ||
| api.Compile(*resourceOverrideFlag, &override) | ||
| api.Merge(reflect.ValueOf(resource), reflect.ValueOf(override), *versionFlag) | ||
| } | ||
| resource.TargetVersionName = *versionFlag | ||
| resource.SetDefault(&product) | ||
| resource.Properties = resource.AddExtraFields(resource.PropertiesWithExcluded(), nil) | ||
| resource.SetDefault(&product) | ||
| product.Objects = []*api.Resource{&resource} | ||
| } |
There was a problem hiding this comment.
Might want to share the code with the loader here and expose the Load single product function. This is going to be a lot of duplicated code and difficult to maintain
4 participants
The rules introduced here are not set in stone. They're inspired by the proto rules, where there's a
proto_librarythat lives at the top level and takes .proto files as srcs, and language-specific rules likego_proto_librarythat depend on it and perform codegen. Here we havemm_productwhich takes theproduct.yaml, andtpg_productwhich generates TPG source from it (and in the future,tgc_productetc.).The BUILD rules need to be able to specify exact locations and names of generated files, and the current MM code assumes that it's building the standard provider tree. The changes to the compiler are to allow the rules to do that w/o disrupting the current build system.
It's still early days and this only does generation and not compilation, but you can now do: