Skip to content

Commit 57aa4a2

Browse files
authored
feat(python): support proto-only APIs (#4075)
Adds handling for "proto-only" APIs (by which we mean APIs without a GAPIC library configured). These APIs: - Use the python and pyi protoc plugins, not gapic - Generate to a slightly different staging directory - Copy the .proto files as well as the generated code This change configures these APIs during migration. Fixes #3204.
1 parent e323d45 commit 57aa4a2

6 files changed

Lines changed: 195 additions & 23 deletions

File tree

doc/config-schema.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ This document describes the schema for the librarian.yaml.
103103

104104
## DartPackage Configuration
105105

106-
[Link to code](../internal/config/language.go#L284)
106+
[Link to code](../internal/config/language.go#L288)
107107
| Field | Type | Description |
108108
| :--- | :--- | :--- |
109109
| `api_keys_environment_variables` | string | APIKeysEnvironmentVariables is a comma-separated list of environment variable names that can contain API keys (e.g., "GOOGLE_API_KEY,GEMINI_API_KEY"). |
@@ -152,6 +152,7 @@ This document describes the schema for the librarian.yaml.
152152
| :--- | :--- | :--- |
153153
| `opt_args` | list of string | OptArgs contains additional options passed to the generator, where the options are common to all apis. Example: ["warehouse-package-name=google-cloud-batch"] |
154154
| `opt_args_by_api` | map[string][]string | OptArgsByAPI contains additional options passed to the generator, where the options vary by api. In each entry, the key is the api (API path) and the value is the list of options to pass when generating that API. Example: {"google/cloud/secrets/v1beta": ["python-gapic-name=secretmanager"]} |
155+
| `proto_only_apis` | list of string | ProtoOnlyAPIs contains the list of API paths which are proto-only, so should use regular protoc Python generation instead of GAPIC. |
155156

156157
## RustCrate Configuration
157158

internal/config/language.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ type PythonPackage struct {
278278
// that API.
279279
// Example: {"google/cloud/secrets/v1beta": ["python-gapic-name=secretmanager"]}
280280
OptArgsByAPI map[string][]string `yaml:"opt_args_by_api,omitempty"`
281+
282+
// ProtoOnlyAPIs contains the list of API paths which are proto-only, so
283+
// should use regular protoc Python generation instead of GAPIC.
284+
ProtoOnlyAPIs []string `yaml:"proto_only_apis,omitempty"`
281285
}
282286

283287
// DartPackage contains Dart-specific library configuration.

internal/librarian/python/generate.go

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"os/exec"
2323
"path/filepath"
24+
"slices"
2425
"strings"
2526

2627
"github.com/googleapis/librarian/internal/config"
@@ -101,7 +102,8 @@ func generateAPI(ctx context.Context, api *config.API, library *config.Library,
101102
// TODO(https://github.com/googleapis/librarian/issues/3210): generate
102103
// directly in place.
103104

104-
stagingChildDirectory := getStagingChildDirectory(api.Path)
105+
protoOnly := isProtoOnly(api, library)
106+
stagingChildDirectory := getStagingChildDirectory(api.Path, protoOnly)
105107
stagingDir := filepath.Join(repoRoot, "owl-bot-staging", library.Name, stagingChildDirectory)
106108
if err := os.MkdirAll(stagingDir, 0755); err != nil {
107109
return err
@@ -141,10 +143,42 @@ func generateAPI(ctx context.Context, api *config.API, library *config.Library,
141143
return fmt.Errorf("%s: %w", cmd.String(), err)
142144
}
143145

146+
// Copy the proto files as well as the generated code for proto-only libraries.
147+
if protoOnly {
148+
if err := stageProtoFiles(googleapisDir, stagingDir, protos); err != nil {
149+
return err
150+
}
151+
}
152+
153+
return nil
154+
}
155+
156+
func stageProtoFiles(googleapisDir, targetDir string, relativeProtoPaths []string) error {
157+
for _, proto := range relativeProtoPaths {
158+
sourceProtoFile := filepath.Join(googleapisDir, proto)
159+
content, err := os.ReadFile(sourceProtoFile)
160+
if err != nil {
161+
return fmt.Errorf("reading proto file %s failed: %w", sourceProtoFile, err)
162+
}
163+
targetProtoFile := filepath.Join(targetDir, proto)
164+
dir := filepath.Dir(targetProtoFile)
165+
if err := os.MkdirAll(dir, 0755); err != nil {
166+
return fmt.Errorf("creating directory %s failed: %w", dir, err)
167+
}
168+
if err := os.WriteFile(targetProtoFile, content, 0644); err != nil {
169+
return fmt.Errorf("writing proto file %s failed: %w", targetProtoFile, err)
170+
}
171+
}
144172
return nil
145173
}
146174

147-
func createProtocOptions(ch *config.API, library *config.Library, googleapisDir, stagingDir string) ([]string, error) {
175+
func createProtocOptions(api *config.API, library *config.Library, googleapisDir, stagingDir string) ([]string, error) {
176+
if isProtoOnly(api, library) {
177+
return []string{
178+
fmt.Sprintf("--python_out=%s", stagingDir),
179+
fmt.Sprintf("--pyi_out=%s", stagingDir),
180+
}, nil
181+
}
148182
// GAPIC library: generate full client library
149183
opts := []string{"metadata"}
150184

@@ -155,7 +189,7 @@ func createProtocOptions(ch *config.API, library *config.Library, googleapisDir,
155189
}
156190
// Then options that apply to this specific api
157191
if library.Python != nil && len(library.Python.OptArgsByAPI) > 0 {
158-
apiOptArgs, ok := library.Python.OptArgsByAPI[ch.Path]
192+
apiOptArgs, ok := library.Python.OptArgsByAPI[api.Path]
159193
if ok {
160194
opts = append(opts, apiOptArgs...)
161195
}
@@ -187,7 +221,7 @@ func createProtocOptions(ch *config.API, library *config.Library, googleapisDir,
187221
}
188222

189223
// Add gRPC service config (retry/timeout settings)
190-
grpcConfigPath, err := serviceconfig.FindGRPCServiceConfig(googleapisDir, ch.Path)
224+
grpcConfigPath, err := serviceconfig.FindGRPCServiceConfig(googleapisDir, api.Path)
191225
if err != nil {
192226
return nil, err
193227
}
@@ -200,12 +234,12 @@ func createProtocOptions(ch *config.API, library *config.Library, googleapisDir,
200234
opts = append(opts, fmt.Sprintf("retry-config=%s", grpcConfigPath))
201235
}
202236

203-
api, err := serviceconfig.Find(googleapisDir, ch.Path, serviceconfig.LangPython)
237+
apiMetadata, err := serviceconfig.Find(googleapisDir, api.Path, serviceconfig.LangPython)
204238
if err != nil {
205239
return nil, err
206240
}
207-
if api != nil && api.ServiceConfig != "" {
208-
opts = append(opts, fmt.Sprintf("service-yaml=%s", api.ServiceConfig))
241+
if apiMetadata != nil && apiMetadata.ServiceConfig != "" {
242+
opts = append(opts, fmt.Sprintf("service-yaml=%s", apiMetadata.ServiceConfig))
209243
}
210244

211245
return []string{
@@ -214,13 +248,17 @@ func createProtocOptions(ch *config.API, library *config.Library, googleapisDir,
214248
}, nil
215249
}
216250

251+
func isProtoOnly(api *config.API, library *config.Library) bool {
252+
return library.Python != nil && slices.Contains(library.Python.ProtoOnlyAPIs, api.Path)
253+
}
254+
217255
// getStagingChildDirectory determines where within owl-bot-staging/{library-name} the
218256
// generated code the given API path should be staged. This is not quite equivalent
219257
// to _get_staging_child_directory in the Python container, as for proto-only directories
220258
// we don't want the apiPath suffix.
221-
func getStagingChildDirectory(apiPath string) string {
259+
func getStagingChildDirectory(apiPath string, isProtoOnly bool) string {
222260
versionCandidate := filepath.Base(apiPath)
223-
if strings.HasPrefix(versionCandidate, "v") {
261+
if strings.HasPrefix(versionCandidate, "v") || isProtoOnly {
224262
return versionCandidate
225263
} else {
226264
return versionCandidate + "-py"

internal/librarian/python/generate_test.go

Lines changed: 120 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515
package python
1616

1717
import (
18+
"errors"
1819
"fmt"
20+
"io/fs"
1921
"os"
2022
"os/exec"
2123
"path/filepath"
24+
"syscall"
2225
"testing"
2326

2427
"github.com/google/go-cmp/cmp"
@@ -31,9 +34,10 @@ const googleapisDir = "../../testdata/googleapis"
3134
func TestGetStagingChildDirectory(t *testing.T) {
3235
t.Parallel()
3336
for _, test := range []struct {
34-
name string
35-
apiPath string
36-
expected string
37+
name string
38+
apiPath string
39+
protoOnly bool
40+
expected string
3741
}{
3842
{
3943
name: "versioned path",
@@ -45,9 +49,15 @@ func TestGetStagingChildDirectory(t *testing.T) {
4549
apiPath: "google/cloud/secretmanager/type",
4650
expected: "type-py",
4751
},
52+
{
53+
name: "proto-only",
54+
apiPath: "google/cloud/secretmanager/type",
55+
protoOnly: true,
56+
expected: "type",
57+
},
4858
} {
4959
t.Run(test.name, func(t *testing.T) {
50-
got := getStagingChildDirectory(test.apiPath)
60+
got := getStagingChildDirectory(test.apiPath, test.protoOnly)
5161
if diff := cmp.Diff(test.expected, got); diff != "" {
5262
t.Errorf("getStagingChildDirectory(%q) returned diff (-want +got):\n%s", test.apiPath, diff)
5363
}
@@ -188,6 +198,32 @@ func TestCreateProtocOptions(t *testing.T) {
188198
"--python_gapic_opt=metadata,transport=rest,rest-numeric-enums,retry-config=google/cloud/secretmanager/v1/secretmanager_grpc_service_config.json,service-yaml=google/cloud/secretmanager/v1/secretmanager_v1.yaml",
189199
},
190200
},
201+
{
202+
name: "proto-only exists but doesn't include API path",
203+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
204+
library: &config.Library{
205+
Python: &config.PythonPackage{
206+
ProtoOnlyAPIs: []string{"google/cloud/secretmanager/type"},
207+
},
208+
},
209+
expected: []string{
210+
"--python_gapic_out=staging",
211+
"--python_gapic_opt=metadata,rest-numeric-enums,retry-config=google/cloud/secretmanager/v1/secretmanager_grpc_service_config.json,service-yaml=google/cloud/secretmanager/v1/secretmanager_v1.yaml",
212+
},
213+
},
214+
{
215+
name: "proto-only exists and includes API path",
216+
api: &config.API{Path: "google/cloud/secretmanager/type"},
217+
library: &config.Library{
218+
Python: &config.PythonPackage{
219+
ProtoOnlyAPIs: []string{"google/cloud/secretmanager/type"},
220+
},
221+
},
222+
expected: []string{
223+
"--python_out=staging",
224+
"--pyi_out=staging",
225+
},
226+
},
191227
} {
192228
t.Run(test.name, func(t *testing.T) {
193229
got, err := createProtocOptions(test.api, test.library, googleapisDir, "staging")
@@ -202,6 +238,86 @@ func TestCreateProtocOptions(t *testing.T) {
202238
}
203239
}
204240

241+
func TestStageProtoFiles(t *testing.T) {
242+
targetDir := t.TempDir()
243+
// Deliberately not including all proto files (or any non-proto) files here.
244+
relativeProtoPaths := []string{
245+
"google/cloud/gkehub/v1/feature.proto",
246+
"google/cloud/gkehub/v1/membership.proto",
247+
}
248+
if err := stageProtoFiles(googleapisDir, targetDir, relativeProtoPaths); err != nil {
249+
t.Fatal(err)
250+
}
251+
copiedFiles := []string{}
252+
if err := filepath.WalkDir(targetDir, func(path string, d fs.DirEntry, err error) error {
253+
if err != nil {
254+
return err
255+
}
256+
if !d.Type().IsDir() {
257+
relative, err := filepath.Rel(targetDir, path)
258+
if err != nil {
259+
return err
260+
}
261+
copiedFiles = append(copiedFiles, relative)
262+
}
263+
return nil
264+
}); err != nil {
265+
t.Fatal(err)
266+
}
267+
if diff := cmp.Diff(relativeProtoPaths, copiedFiles); diff != "" {
268+
t.Errorf("mismatch (-want +got):\n%s", diff)
269+
}
270+
}
271+
272+
func TestStageProtoFiles_Error(t *testing.T) {
273+
t.Parallel()
274+
for _, test := range []struct {
275+
name string
276+
relativeProtoPaths []string
277+
setup func(t *testing.T, targetDir string)
278+
wantErr error
279+
}{
280+
{
281+
name: "path doesn't exist",
282+
relativeProtoPaths: []string{"google/cloud/bogus.proto"},
283+
wantErr: os.ErrNotExist,
284+
},
285+
{
286+
name: "can't create directory",
287+
relativeProtoPaths: []string{"google/cloud/gkehub/v1/feature.proto"},
288+
setup: func(t *testing.T, targetDir string) {
289+
// Create a file with the name of the directory we'd create.
290+
if err := os.WriteFile(filepath.Join(targetDir, "google"), []byte{}, 0644); err != nil {
291+
t.Fatal(err)
292+
}
293+
},
294+
wantErr: syscall.ENOTDIR,
295+
},
296+
{
297+
name: "can't write file",
298+
relativeProtoPaths: []string{"google/cloud/gkehub/v1/feature.proto"},
299+
setup: func(t *testing.T, targetDir string) {
300+
// Create a directory with the name of the file we'd create.
301+
if err := os.MkdirAll(filepath.Join(targetDir, "google", "cloud", "gkehub", "v1", "feature.proto"), 0755); err != nil {
302+
t.Fatal(err)
303+
}
304+
},
305+
wantErr: syscall.EISDIR,
306+
},
307+
} {
308+
t.Run(test.name, func(t *testing.T) {
309+
targetDir := t.TempDir()
310+
if test.setup != nil {
311+
test.setup(t, targetDir)
312+
}
313+
gotErr := stageProtoFiles(googleapisDir, targetDir, test.relativeProtoPaths)
314+
if !errors.Is(gotErr, test.wantErr) {
315+
t.Errorf("stageProtoFiles error = %v, wantErr %v", gotErr, test.wantErr)
316+
}
317+
})
318+
}
319+
}
320+
205321
func TestCopyReadmeToDocsDir(t *testing.T) {
206322
t.Parallel()
207323
for _, test := range []struct {

tool/cmd/migrate/python.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"encoding/json"
1919
"fmt"
2020
"io/fs"
21-
"log/slog"
2221
"os"
2322
"path/filepath"
2423
"regexp"
@@ -108,15 +107,14 @@ func applyBuildBazelConfig(library *config.Library, googleapisDir string) (*conf
108107
}
109108
allTransports := make(map[string]bool)
110109
transportsByApi := make(map[string]string)
111-
allGapic := true
112110

113111
for _, api := range library.APIs {
114112
bazelGapicInfo, err := parseBazelPythonInfo(googleapisDir, api.Path)
115113
if err != nil {
116114
return nil, err
117115
}
118116
if bazelGapicInfo == nil {
119-
allGapic = false
117+
pythonConfig.ProtoOnlyAPIs = append(pythonConfig.ProtoOnlyAPIs, api.Path)
120118
continue
121119
}
122120
transportsByApi[api.Path] = bazelGapicInfo.transport
@@ -125,26 +123,33 @@ func applyBuildBazelConfig(library *config.Library, googleapisDir string) (*conf
125123
pythonConfig.OptArgsByAPI[api.Path] = bazelGapicInfo.optArgs
126124
}
127125
}
128-
if !allGapic {
129-
slog.Info("Skipping not-fully-GAPIC library", "library", library.Name)
130-
return nil, nil
131-
}
132126
if len(allTransports) == 1 {
133127
// One consistent transport; set it library-wide if it's not the default.
128+
// This assumes that where there's a mixture of GAPIC and non-GAPIC, the
129+
// first path is a GAPIC API, but that happens to be true for now (and
130+
// we don't care what happens post-migration).
134131
transport := transportsByApi[library.APIs[0].Path]
135132
if transport != "grpc+rest" {
136133
library.Transport = transport
137134
}
138135
} else {
139-
// Transport differs by API version. Add it into OptArgsByAPI.
136+
// Transport differs by API version. Add it into OptArgsByAPI, but only
137+
// for non-proto-only APIs. (Proto-only APIs don't have a transport
138+
// anyway.)
140139
for _, api := range library.APIs {
140+
if slices.Contains(pythonConfig.ProtoOnlyAPIs, api.Path) {
141+
continue
142+
}
141143
optArgs := pythonConfig.OptArgsByAPI[api.Path]
142144
optArgs = append(optArgs, fmt.Sprintf("transport=%s", transportsByApi[api.Path]))
143145
pythonConfig.OptArgsByAPI[api.Path] = optArgs
144146
}
145147
}
146148

147-
if len(pythonConfig.OptArgsByAPI) > 0 {
149+
if len(pythonConfig.OptArgsByAPI) > 0 || len(pythonConfig.ProtoOnlyAPIs) > 0 {
150+
if len(pythonConfig.OptArgsByAPI) == 0 {
151+
pythonConfig.OptArgsByAPI = nil
152+
}
148153
library.Python = pythonConfig
149154
}
150155
return library, nil

tool/cmd/migrate/python_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ func TestBuildPythonLibraries(t *testing.T) {
107107
librarianConfig: &legacyconfig.LibrarianConfig{},
108108
},
109109
want: []*config.Library{
110+
{
111+
Name: "google-cloud-audit-log",
112+
APIs: []*config.API{{Path: "google/cloud/audit"}},
113+
ReleaseLevel: "preview",
114+
Python: &config.PythonPackage{
115+
ProtoOnlyAPIs: []string{"google/cloud/audit"},
116+
},
117+
},
110118
{
111119
Name: "google-cloud-workstations",
112120
ReleaseLevel: "preview",

0 commit comments

Comments
 (0)