Skip to content
Permalink
Browse files
fix(internal/gapicgen): use correct region tags for gensnippets (#4022)
A side effect of this is that only methods that correspond to RPCs get
region tags.

I changed the error handling so any snippet issues are only warnings and
don't `log.Fatal` (cc @noahdietz).

I deleted and regenerated all of the snippets, which led to non-Method
snippets (e.g. `NewClient`-type functions) being deleted.

The first commit has the actual generator changes. The second commit is
the regeneration.

cc @amanda-tarafa.
  • Loading branch information
tbpg committed Apr 29, 2021
1 parent 022fc35 commit 8ccd689cab08f016008ca06a939a4828817d4a25
Showing 2,453 changed files with 3,847 additions and 26,768 deletions.
@@ -111,7 +111,7 @@ func (g *GapicGenerator) RegenSnippets(ctx context.Context) error {
return err
}
if err := gensnippets.Generate(g.googleCloudDir, snippetDir, apiShortnames); err != nil {
return fmt.Errorf("error generating snippets: %v", err)
log.Printf("warning: got the following non-fatal errors generating snippets: %v", err)
}
if err := replaceAllForSnippets(g.googleCloudDir, snippetDir); err != nil {
return err
@@ -31,6 +31,8 @@ import (
"cloud.google.com/go/internal/godocfx/pkgload"
"cloud.google.com/go/third_party/go/doc"
"golang.org/x/sys/execabs"
"google.golang.org/genproto/googleapis/gapic/metadata"
"google.golang.org/protobuf/encoding/protojson"
)

// Generate reads all modules in rootDir and outputs their examples in outDir.
@@ -68,13 +70,13 @@ func Generate(rootDir, outDir string, apiShortnames map[string]string) error {
return fmt.Errorf("failed to load packages: %v", err)
}
for _, pi := range pis {
if err := processExamples(pi.Doc, pi.Fset, trimPrefix, outDir, apiShortnames); err != nil {
errs = append(errs, fmt.Errorf("failed to process examples: %v", err))
if eErrs := processExamples(pi.Doc, pi.Fset, trimPrefix, rootDir, outDir, apiShortnames); len(eErrs) > 0 {
errs = append(errs, fmt.Errorf("%v", eErrs))
}
}
}
if len(errs) > 0 {
log.Fatal(errs)
return fmt.Errorf("example errors: %v", errs)
}

if len(dirs) > 0 {
@@ -103,69 +105,90 @@ var skip = map[string]bool{
"cloud.google.com/go/translate": true, // Has newer version.
}

func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, outDir string, apiShortnames map[string]string) error {
func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, rootDir, outDir string, apiShortnames map[string]string) []error {
if skip[pkg.ImportPath] {
return nil
}
trimmed := strings.TrimPrefix(pkg.ImportPath, trimPrefix)
regionTags, err := computeRegionTags(rootDir, trimmed, apiShortnames)
if err != nil {
return []error{err}
}
if len(regionTags) == 0 {
// Nothing to do.
return nil
}
outDir = filepath.Join(outDir, trimmed)

shortname, ok := apiShortnames[pkg.ImportPath]
if !ok {
// Do our best to find a shortname. For example,
// cloud.google.com/go/bigtable/bttest should lead to
// cloud.google.com/go/bigtable.
bestMatch := ""
for path := range apiShortnames {
if strings.HasPrefix(pkg.ImportPath, path) {
if len(path) > len(bestMatch) {
bestMatch = path
}
// Note: only process methods because they correspond to RPCs.

var errs []error
for _, t := range pkg.Types {
for _, m := range t.Methods {
if len(m.Examples) == 0 {
// Nothing to do for this method.
continue
}
dir := filepath.Join(outDir, t.Name, m.Name)
regionTag, ok := regionTags[t.Name][m.Name]
if !ok {
errs = append(errs, fmt.Errorf("could not find region tag for %s %s.%s", pkg.ImportPath, t.Name, m.Name))
continue
}
if err := writeExamples(dir, m.Examples, fset, regionTag); err != nil {
errs = append(errs, err)
}
}
if bestMatch == "" {
return fmt.Errorf("could not find API shortname for %v", pkg.ImportPath)
}
log.Printf("The best match for %q is %q", pkg.ImportPath, bestMatch)
shortname = apiShortnames[bestMatch]
}
regionTag := shortname + "_generated" + strings.ReplaceAll(trimmed, "/", "_")

// Note: variables and constants don't have examples.
return errs
}

for _, f := range pkg.Funcs {
dir := filepath.Join(outDir, f.Name)
if err := writeExamples(dir, f.Examples, fset, regionTag); err != nil {
return err
}
// computeRegionTags gets the region tags for the given path, keyed by client name and method name.
func computeRegionTags(rootDir, path string, apiShortnames map[string]string) (regionTags map[string]map[string]string, err error) {
metadataPath := filepath.Join(rootDir, path, "gapic_metadata.json")
f, err := os.ReadFile(metadataPath)
if err != nil {
// If there is no gapic_metadata.json file, don't generate snippets.
// This isn't an error, though, because some packages aren't GAPICs and
// shouldn't get snippets in the first place.
return nil, nil
}
m := metadata.GapicMetadata{}
if err := protojson.Unmarshal(f, &m); err != nil {
return nil, err
}
shortname, ok := apiShortnames[m.GetLibraryPackage()]
if !ok {
return nil, fmt.Errorf("could not find shortname for %q", m.GetLibraryPackage())
}
protoParts := strings.Split(m.GetProtoPackage(), ".")
apiVersion := protoParts[len(protoParts)-1]

for _, t := range pkg.Types {
dir := filepath.Join(outDir, t.Name)
if err := writeExamples(dir, t.Examples, fset, regionTag); err != nil {
return err
}
for _, f := range t.Funcs {
fDir := filepath.Join(dir, f.Name)
if err := writeExamples(fDir, f.Examples, fset, regionTag); err != nil {
return err
}
}
for _, m := range t.Methods {
mDir := filepath.Join(dir, m.Name)
if err := writeExamples(mDir, m.Examples, fset, regionTag); err != nil {
return err
regionTags = map[string]map[string]string{}
for sName, s := range m.GetServices() {
for _, c := range s.GetClients() {
for rpc, methods := range c.GetRpcs() {
if len(methods.GetMethods()) != 1 {
return nil, fmt.Errorf("%s %s %s found %d methods", m.GetLibraryPackage(), sName, c.GetLibraryClient(), len(methods.GetMethods()))
}
if methods.GetMethods()[0] != rpc {
return nil, fmt.Errorf("%s %s %s %q does not match %q", m.GetLibraryPackage(), sName, c.GetLibraryClient(), methods.GetMethods()[0], rpc)
}

// Every Go method is synchronous.
regionTag := fmt.Sprintf("%s_%s_generated_%s_%s_sync", shortname, apiVersion, sName, rpc)

if regionTags[c.GetLibraryClient()] == nil {
regionTags[c.GetLibraryClient()] = map[string]string{}
}
regionTags[c.GetLibraryClient()][rpc] = regionTag
}
}
}
return nil
return regionTags, nil
}

func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regionTag string) error {
if len(exs) == 0 {
// Nothing to do.
return nil
}
for _, ex := range exs {
dir := outDir
if len(exs) > 1 {
@@ -207,7 +230,12 @@ func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regio
if _, err := f.WriteString(header()); err != nil {
return err
}
tag := regionTag + "_" + ex.Name

tag := regionTag
if len(ex.Suffix) > 0 {
tag += "_" + ex.Suffix
}

// Include an extra newline to keep separate from the package declaration.
if _, err := fmt.Fprintf(f, "// [START %v]\n\n", tag); err != nil {
return err
@@ -14,6 +14,8 @@ require (
golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20210415045647-66c3f260301c
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1
google.golang.org/protobuf v1.26.0
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/src-d/go-git.v4 v4.13.1
gopkg.in/yaml.v2 v2.4.0
@@ -477,6 +477,7 @@ google.golang.org/genproto v0.0.0-20210222152913-aa3ee6e6a81c/go.mod h1:FWY/as6D
google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210310155132-4ce2db91004e/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210319143718-93e7006c17a6/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1 h1:E7wSQBXkH3T3diucK+9Z1kjn4+/9tNG7lZLr75oOhh8=
google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1/go.mod h1:9lPAdzaEmUacj36I+k7YKbEc5CXzPIeORRgDAUOu28A=
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]

package main

@@ -39,4 +39,4 @@ func main() {
}
}

// [END accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
// [START accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
// [END accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
// [START accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]

package main

@@ -51,4 +51,4 @@ func main() {
}
}

// [END accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
// [END accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]

This file was deleted.

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
// [START accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
// [END accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]

package main

@@ -51,4 +51,4 @@ func main() {
}
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]

package main

@@ -43,4 +43,4 @@ func main() {
_ = resp
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]

package main

@@ -39,4 +39,4 @@ func main() {
}
}

// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]
Loading

0 comments on commit 8ccd689

Please sign in to comment.