Skip to content
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(bazel): detach spring bazel rules #1065

Merged
merged 8 commits into from
Nov 16, 2022

Conversation

diegomarquezp
Copy link
Contributor

@diegomarquezp diegomarquezp commented Oct 23, 2022

This allows spring autoconfig generation to be called from a new rule "java_gapic_spring_library" that is independent from java_gapic_library.

NOTE: The following instructions are for local tests only. Definitive modifications in googleapis are mainly made from rules_gapic's BUILD file generator

For example, in googleapis/google/cloud/vision/v1, we can modify BUILD.bzl by adding:

load(
    "@com_google_googleapis_imports//:imports.bzl",
    "java_gapic_assembly_gradle_pkg",
    "java_gapic_library",
    "java_gapic_spring_library", # defined and explained below
    "java_gapic_test",
    "java_grpc_library",
    "java_proto_library",
)
...
java_gapic_spring_library(
    name = "vision_java_gapic_spring",
    srcs = [":vision_proto_with_info"],
    gapic_yaml = "vision_gapic.yaml",
    grpc_service_config = "vision_grpc_service_config.json",
    service_yaml = "vision_v1.yaml",
)

In googleapis/repository_rules.bzl we need to add a switch and enable spring rules from gapic-generator-java/rules_java_gapic/java_gapic_spring.bzl:

    rules["java_gapic_spring_library"] = _switch(
        java and grpc and gapic,
        "@gapic_generator_java//rules_java_gapic:java_gapic_spring.bzl",
    )

Then build from googleapis:

bazel build //google/cloud/vision/v1:vision_java_gapic_spring

Which will generate the files:

vision_java_gapic_spring_raw.srcjar
vision_java_gapic_spring_raw.srcjar.zip
vision_java_gapic_spring-spring.srcjar # formatted

@diegomarquezp diegomarquezp requested review from a team as code owners October 23, 2022 22:21
@diegomarquezp diegomarquezp force-pushed the autoconfig-gen-draft2-bazel branch from 45e02fd to d89f03e Compare October 26, 2022 16:22
@diegomarquezp diegomarquezp force-pushed the autoconfig-gen-draft2-bazel branch from d89f03e to e37004f Compare November 8, 2022 19:45
@diegomarquezp
Copy link
Contributor Author

Force pushed rebase on current autoconfig-gen-draft2

Copy link
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two quick notes below.
And a more general question for @blakeli0: about the resources jar processing portion in java_gapic_library here that Diego did not bring into spring rule: It seems to me it's only needed for client library to be used in java_gapic_pkg.bzl steps to produce a gapic-[package name]-java-resources.tar.gz with a build.gradle. IIUC that gradle file is not used in the post process of generating pom file, does it mean that all related logic can be potentially cleaned-up in gapic-generator itself in future?

grpc_service_config = None,
gapic_yaml = None,
service_yaml = None,
**kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this is derived from _java_gapic_library() in java_gapic.bzl and you removed a couple of options. Can you double check if they don't affect sping-codegen for us?
E.g. I noticed transport is removed, If I get it right, this is where transport is specified as grpc or grpc+rest or rest, which get parsed in and eventually plays a part in #1078. Are we losing this info here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, we indeed need transport to be specified! I'll double check the impact of the other options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the other option is rest_numeric_enums, which is used by HttpJsonServiceStubClassComposer to determine how to serialize enums (by number or value). I see that our spring composers do not mention enums, so it should be safe to remove.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably not in scope now, but once SpringCodeGen decide to support both grpc and rest, we need a way to pass the correct value to this Spring specific bazel rule, and currently the only source of truth is the Bazel files in googleapis.

BUILD.bazel Show resolved Hide resolved
@blakeli0
Copy link
Collaborator

Two quick notes below. And a more general question for @blakeli0: about the resources jar processing portion in java_gapic_library here that Diego did not bring into spring rule: It seems to me it's only needed for client library to be used in java_gapic_pkg.bzl steps to produce a gapic-[package name]-java-resources.tar.gz with a build.gradle. IIUC that gradle file is not used in the post process of generating pom file, does it mean that all related logic can be potentially cleaned-up in gapic-generator itself in future?

The generated Gradle file are used by self-services client libraries(Ads, Geo etc.), so we are not going to remove them in the near term. However, we are trying to simplify the process by using the generator as a jar.

@diegomarquezp diegomarquezp force-pushed the autoconfig-gen-draft2-bazel branch from 808c778 to 767fee9 Compare November 15, 2022 21:24
rules_java_gapic/java_gapic_spring.bzl Outdated Show resolved Hide resolved
@emmileaf emmileaf added the spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch. label Nov 16, 2022
Co-authored-by: Emily Wang <emmwang@google.com>
@diegomarquezp diegomarquezp merged commit f877870 into autoconfig-gen-draft2 Nov 16, 2022
@diegomarquezp diegomarquezp deleted the autoconfig-gen-draft2-bazel branch November 16, 2022 20:12
@sonarcloud
Copy link

sonarcloud bot commented Nov 16, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

zhumin8 added a commit that referenced this pull request Nov 21, 2022
…tatic types instead of vapor references (#1046)

Adding Spring related dependencies to build files and use static types instead of vapor references. 
Benefit of this change:
- easier to test
- less code change needed for future changes (e.g. package change of classes).

One caveat is that because `gapic-generator-java` is brought into `googleapis` as `http_archive` code addition to `googleapis` is also needed to include these libraries. Add below code snippet to [WORKSPACE](https://github.com/googleapis/googleapis/blob/38e8b447d173909d3b2fe8fdc3e6cbb3c85442fd/WORKSPACE#L239-L245):
```
SPRING_MAVEN_ARTIFACTS = [
    "org.springframework.boot:spring-boot-starter:2.7.4",
    "com.google.cloud:spring-cloud-gcp-core:3.3.0",
]

maven_install(
    artifacts = PROTOBUF_MAVEN_ARTIFACTS + SPRING_MAVEN_ARTIFACTS,
    generate_compat_repositories = True,
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)

```

------------
Updates:
- Updated pr with #1065, 
- Moved annotation classed added in #1045 to static types.
- Fixed conflicts from merged changes in #1093
- Moved logging classes added in #1053 to static types.
suztomo pushed a commit that referenced this pull request Mar 21, 2023
#1065)

* chore: copy functionality from io.grpc.internal.SharedResourceHolder

Included related classes and test file. Tests were adapted to use an
easymock mock

* fix: remove grpc-core from deps

* fix: add used undeclared dependency jsr305

* fix: add copy explanation comments, remove `@ThreadSafe` and related dependency

* Update google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/LogExceptionRunnable.java

Co-authored-by: Mike Eltsufin <meltsufin@google.com>

* Update google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/SharedResourceHolder.java

Co-authored-by: Mike Eltsufin <meltsufin@google.com>

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Mike Eltsufin <meltsufin@google.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spring pr that's related to spring code gen, intend to merge into autoconfig-gen-draft2 branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants