-
Notifications
You must be signed in to change notification settings - Fork 49
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
feat: DIREGAPIC initial implementation #746
Changes from 2 commits
8983e23
d568db5
8de3877
b1d0660
72a53c6
2dcf807
d99d8d0
35b6dde
393840e
eaf7916
dc8a156
f0563bb
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 |
---|---|---|
|
@@ -127,12 +127,14 @@ def java_gapic_library( | |
service_yaml = None, | ||
deps = [], | ||
test_deps = [], | ||
transport = None, | ||
miraleung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
java_generator_name = "java_gapic", | ||
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 is the motivation for the new 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. To reuse the same rule for dumping and running from a file (i.e. substitute Main with MainDump). 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. SG, could we please add some documentation here? And since this is intended only for our use, could we please make this fail if the string does not match the generator or the dumpers' values? 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. Renamed the parameter to _java_generator_name (starting with udnerscore), to indicate that it is a private thing. Can we please keep it flexible about which values can be passed (I rather consider it to be a feature). 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. Sure, although some documentation would still be ideal. Could you please check if the commits have been pushed? I'm still seeing the param without the leading underscore. |
||
**kwargs): | ||
file_args_dict = {} | ||
|
||
if grpc_service_config: | ||
file_args_dict[grpc_service_config] = "grpc-service-config" | ||
else: | ||
elif transport != "rest": | ||
for keyword in NO_GRPC_CONFIG_ALLOWLIST: | ||
if keyword not in name: | ||
fail("Missing a gRPC service config file") | ||
|
@@ -157,21 +159,25 @@ def java_gapic_library( | |
srcjar_name = name + "_srcjar" | ||
raw_srcjar_name = srcjar_name + "_raw" | ||
output_suffix = ".srcjar" | ||
opt_args = [] | ||
|
||
if transport: | ||
opt_args.append("transport=%s" % transport) | ||
|
||
# Produces the GAPIC metadata file if this flag is set. to any value. | ||
# Protoc invocation: --java_gapic_opt=metadata | ||
plugin_args = ["metadata"] | ||
|
||
_java_generator_name = "java_gapic" | ||
proto_custom_library( | ||
name = raw_srcjar_name, | ||
deps = srcs, | ||
plugin = Label("@gapic_generator_java//:protoc-gen-%s" % _java_generator_name), | ||
plugin = Label("@gapic_generator_java//:protoc-gen-%s" % java_generator_name), | ||
plugin_args = plugin_args, | ||
plugin_file_args = {}, | ||
opt_file_args = file_args_dict, | ||
output_type = _java_generator_name, | ||
output_type = java_generator_name, | ||
output_suffix = output_suffix, | ||
opt_args = opt_args, | ||
**kwargs | ||
) | ||
|
||
|
@@ -201,10 +207,7 @@ def java_gapic_library( | |
"@com_google_protobuf//:protobuf_java", | ||
"@com_google_api_api_common//jar", | ||
"@com_google_api_gax_java//gax:gax", | ||
"@com_google_api_gax_java//gax-grpc:gax_grpc", | ||
"@com_google_guava_guava//jar", | ||
"@io_grpc_grpc_java//core:core", | ||
"@io_grpc_grpc_java//protobuf:protobuf", | ||
"@com_google_code_findbugs_jsr305//jar", | ||
"@org_threeten_threetenbp//jar", | ||
"@io_opencensus_opencensus_api//jar", | ||
|
@@ -214,6 +217,17 @@ def java_gapic_library( | |
"@javax_annotation_javax_annotation_api//jar", | ||
] | ||
|
||
if transport == "rest": | ||
actual_deps += [ | ||
"@com_google_api_gax_java//gax-httpjson:gax_httpjson", | ||
] | ||
else: | ||
actual_deps += [ | ||
"@com_google_api_gax_java//gax-grpc:gax_grpc", | ||
"@io_grpc_grpc_java//core:core", | ||
"@io_grpc_grpc_java//protobuf:protobuf", | ||
] | ||
|
||
native.java_library( | ||
name = name, | ||
srcs = ["%s.srcjar" % srcjar_name], | ||
|
@@ -224,15 +238,24 @@ def java_gapic_library( | |
# Test deps. | ||
actual_test_deps = [ | ||
"@com_google_googleapis//google/type:type_java_proto", # Commonly used. | ||
"@com_google_api_gax_java//gax-grpc:gax_grpc_testlib", | ||
"@com_google_api_gax_java//gax:gax_testlib", | ||
"@com_google_code_gson_gson//jar", | ||
"@io_grpc_grpc_java//auth:auth", | ||
"@io_grpc_grpc_netty_shaded//jar", | ||
"@io_grpc_grpc_java//stub:stub", | ||
"@io_opencensus_opencensus_contrib_grpc_metrics//jar", | ||
"@junit_junit//jar", | ||
] | ||
|
||
if transport == "rest": | ||
actual_test_deps += [ | ||
"@com_google_api_gax_java//gax-httpjson:gax_httpjson_testlib", | ||
] | ||
else: | ||
actual_test_deps += [ | ||
"@com_google_api_gax_java//gax-grpc:gax_grpc_testlib", | ||
"@io_grpc_grpc_java//auth:auth", | ||
"@io_grpc_grpc_netty_shaded//jar", | ||
"@io_grpc_grpc_java//stub:stub", | ||
"@io_opencensus_opencensus_contrib_grpc_metrics//jar", | ||
] | ||
|
||
_append_dep_without_duplicates(actual_test_deps, test_deps) | ||
_append_dep_without_duplicates(actual_test_deps, actual_deps) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
buildscript { | ||
repositories { | ||
mavenCentral() | ||
} | ||
} | ||
|
||
apply plugin: 'java' | ||
|
||
description = 'GAPIC library for {{name}}' | ||
group = 'com.google.cloud' | ||
version = (findProperty('version') == 'unspecified') ? '0.0.0-SNAPSHOT' : version | ||
sourceCompatibility = 1.7 | ||
targetCompatibility = 1.7 | ||
|
||
repositories { | ||
mavenCentral() | ||
mavenLocal() | ||
} | ||
|
||
compileJava.options.encoding = 'UTF-8' | ||
javadoc.options.encoding = 'UTF-8' | ||
|
||
dependencies { | ||
compile 'com.google.api:gax:{{version.gax}}' | ||
testCompile 'com.google.api:gax:{{version.gax}}:testlib' | ||
compile 'com.google.api:gax-httpjson:{{version.gax_httpjson}}' | ||
testCompile 'com.google.api:gax-httpjson:{{version.gax_httpjson}}:testlib' | ||
testCompile '{{maven.junit_junit}}' | ||
{{extra_deps}} | ||
} | ||
|
||
task smokeTest(type: Test) { | ||
filter { | ||
includeTestsMatching "*SmokeTest" | ||
setFailOnNoMatchingTests false | ||
} | ||
} | ||
|
||
test { | ||
exclude "**/*SmokeTest*" | ||
} | ||
|
||
sourceSets { | ||
main { | ||
java { | ||
srcDir 'src/main/java' | ||
} | ||
} | ||
} | ||
|
||
clean { | ||
delete 'all-jars' | ||
} | ||
|
||
task allJars(type: Copy) { | ||
dependsOn test, jar | ||
into 'all-jars' | ||
// Replace with `from configurations.testRuntime, jar` to include test dependencies | ||
from configurations.runtime, jar | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// Copyright 2021 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package com.google.api.generator; | ||
|
||
import com.google.api.AnnotationsProto; | ||
import com.google.api.ClientProto; | ||
import com.google.api.FieldBehaviorProto; | ||
import com.google.api.ResourceProto; | ||
import com.google.longrunning.OperationsProto; | ||
import com.google.protobuf.ExtensionRegistry; | ||
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest; | ||
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorResponse; | ||
import java.io.IOException; | ||
|
||
public class MainDumper { | ||
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. This will be really useful. Could we move this into a debug-only subpackage (to separate this from the main generator logic) and rename this class to something like 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. Removed from this PR, will add in a separate one, incorporating the PR feedback. |
||
public static void main(String[] args) throws IOException { | ||
ExtensionRegistry registry = ExtensionRegistry.newInstance(); | ||
registerAllExtensions(registry); | ||
CodeGeneratorRequest request = CodeGeneratorRequest.parseFrom(System.in, registry); | ||
|
||
CodeGeneratorResponse.Builder response = CodeGeneratorResponse.newBuilder(); | ||
response | ||
.setSupportedFeatures(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL_VALUE) | ||
.addFileBuilder() | ||
.setName("desc-dump.bin") | ||
.setContentBytes(request.toByteString()); | ||
response.build().writeTo(System.out); | ||
} | ||
|
||
/** Register all extensions needed to process API protofiles. */ | ||
private static void registerAllExtensions(ExtensionRegistry extensionRegistry) { | ||
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. Since we're doing this in three places now, can we centralize this? We can have a minimal standalone class (e.g. in a 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. Removed from this PR, will add in a separate one, incorporating the PR feedback. |
||
OperationsProto.registerAllExtensions(extensionRegistry); | ||
AnnotationsProto.registerAllExtensions(extensionRegistry); | ||
ClientProto.registerAllExtensions(extensionRegistry); | ||
ResourceProto.registerAllExtensions(extensionRegistry); | ||
FieldBehaviorProto.registerAllExtensions(extensionRegistry); | ||
} | ||
} |
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.
Since this is independent of DIREGAPIC, could we please split this out into a separate PR?
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.
Removed from this PR, will add in a separate one, incorporating the PR feedback.