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

fix(bazel): Remove monolith rule deps from the Java µgen Bazel rules [ggj] #764

Merged
merged 14 commits into from
Jun 17, 2021

Conversation

miraleung
Copy link
Contributor

@miraleung miraleung commented Jun 10, 2021

This PR is needed for the preliminary removal of the monolith rules from googleapis' switched_rules_by_language.

@miraleung miraleung requested a review from a team as a code owner June 10, 2021 20:44
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 10, 2021
@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #764 (60c50b3) into master (1294c29) will not change coverage.
The diff coverage is n/a.

❗ Current head 60c50b3 differs from pull request most recent head 6e093e9. Consider uploading reports for the commit 6e093e9 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #764   +/-   ##
=======================================
  Coverage   91.25%   91.25%           
=======================================
  Files         140      140           
  Lines       14884    14884           
  Branches     1061     1061           
=======================================
  Hits        13582    13582           
  Misses       1011     1011           
  Partials      291      291           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1294c29...6e093e9. Read the comment docs.

@miraleung miraleung requested a review from vam-google June 10, 2021 22:18
@miraleung miraleung marked this pull request as draft June 10, 2021 22:55
@miraleung miraleung marked this pull request as ready for review June 15, 2021 22:29
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
"asset": "@com_google_googleapis//google/cloud/asset/v1:asset_java_gapic",
"credentials": "@com_google_googleapis//google/iam/credentials/v1:credentials_java_gapic",
"asset": ":asset_java_gapic",
"credentials": ":credentials_java_gapic",
"iam": ":iam_java_gapic", # Googleapis' LRO does not have a Java Gapic.
"kms": ":kms_java_gapic", # Local target because mixins are not rolled out yet.
"pubsub": ":pubsub_java_gapic",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are some tests coming from googleapis/googleapis-discovery and some are "native" to gapic-generator-java? (i.e. what is special about iam, kms and pubsub?)

Copy link
Contributor Author

@miraleung miraleung Jun 17, 2021

Choose a reason for hiding this comment

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

I had to add some custom changes to IAM, KMS, and PubSub to exercise edge cases that the googleapis target does not cover.

.setTotalTimeout(Duration.ofMillis(60000L))
.build();
definitions.put("retry_policy_0_params", settings);
settings = RetrySettings.newBuilder().setRpcTimeoutMultiplier(1.0).build();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this getting removed? Seems like something that would happen when switching from monolith to microgen, or loosing grpc_service_config somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks for catching that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On testing again - this will now fail because of the monolith rule removal, since it runs all the non-golden tests from the googleapis' targets. Will make this change on my next pass of removing the monolith's rules entirely from this codebase.

Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

LGTM, but please check the bazel test //... topic comment.

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@miraleung miraleung merged commit bff3efc into master Jun 17, 2021
@miraleung miraleung deleted the fix/bazel_monolith branch June 17, 2021 21:11
suztomo pushed a commit that referenced this pull request Mar 21, 2023
suztomo pushed a commit that referenced this pull request Mar 21, 2023
🤖 I have created a release *beep* *boop*
---


## [3.0.0](googleapis/java-shared-dependencies@v2.13.0...v3.0.0) (2022-07-29)


### Bug Fixes

* enable longpaths support for windows test ([#1485](https://github.com/googleapis/java-shared-dependencies/issues/1485)) ([#738](googleapis/java-shared-dependencies#738)) ([48b157d](googleapis/java-shared-dependencies@48b157d))


### Dependencies

* update dependency com.google.api-client:google-api-client-bom to v1.35.2 ([#729](googleapis/java-shared-dependencies#729)) ([d518319](googleapis/java-shared-dependencies@d518319))
* update dependency com.google.api-client:google-api-client-bom to v2 ([#746](googleapis/java-shared-dependencies#746)) ([ef2b57a](googleapis/java-shared-dependencies@ef2b57a))
* update dependency com.google.api.grpc:grpc-google-common-protos to v2.9.2 ([#741](googleapis/java-shared-dependencies#741)) ([347269f](googleapis/java-shared-dependencies@347269f))
* update dependency com.google.auth:google-auth-library-bom to v1.8.0 ([#726](googleapis/java-shared-dependencies#726)) ([236bbb3](googleapis/java-shared-dependencies@236bbb3))
* update dependency com.google.auth:google-auth-library-bom to v1.8.1 ([#742](googleapis/java-shared-dependencies#742)) ([dabdbdf](googleapis/java-shared-dependencies@dabdbdf))
* update dependency com.google.cloud:first-party-dependencies to v2 ([#747](googleapis/java-shared-dependencies#747)) ([93b1ed8](googleapis/java-shared-dependencies@93b1ed8))
* update dependency com.google.cloud:grpc-gcp to v1.2.1 ([#751](googleapis/java-shared-dependencies#751)) ([618b00c](googleapis/java-shared-dependencies@618b00c))
* update dependency com.google.cloud:third-party-dependencies to v2 ([#748](googleapis/java-shared-dependencies#748)) ([afca3fd](googleapis/java-shared-dependencies@afca3fd))
* update dependency com.google.http-client:google-http-client-bom to v1.42.1 ([#730](googleapis/java-shared-dependencies#730)) ([4fdaad8](googleapis/java-shared-dependencies@4fdaad8))
* update dependency com.google.http-client:google-http-client-bom to v1.42.2 ([#749](googleapis/java-shared-dependencies#749)) ([68a82f4](googleapis/java-shared-dependencies@68a82f4))
* update dependency com.google.protobuf:protobuf-bom to v3.21.2 ([#722](googleapis/java-shared-dependencies#722)) ([68f570e](googleapis/java-shared-dependencies@68f570e))
* update dependency com.google.protobuf:protobuf-bom to v3.21.3 ([#756](googleapis/java-shared-dependencies#756)) ([7429507](googleapis/java-shared-dependencies@7429507))
* update dependency com.google.protobuf:protobuf-bom to v3.21.4 ([#759](googleapis/java-shared-dependencies#759)) ([f033db0](googleapis/java-shared-dependencies@f033db0))
* update dependency io.grpc:grpc-bom to v1.48.0 ([#752](googleapis/java-shared-dependencies#752)) ([9678d52](googleapis/java-shared-dependencies@9678d52))
* update dependency org.checkerframework:checker-qual to v3.23.0 ([#736](googleapis/java-shared-dependencies#736)) ([816d380](googleapis/java-shared-dependencies@816d380))
* update gax.version to v2.18.3 ([#731](googleapis/java-shared-dependencies#731)) ([5bbf1e1](googleapis/java-shared-dependencies@5bbf1e1))
* update gax.version to v2.18.4 ([#735](googleapis/java-shared-dependencies#735)) ([5161c6e](googleapis/java-shared-dependencies@5161c6e))
* update gax.version to v2.18.5 ([#758](googleapis/java-shared-dependencies#758)) ([608e040](googleapis/java-shared-dependencies@608e040))
* update gax.version to v2.18.6 ([#763](googleapis/java-shared-dependencies#763)) ([84b81e9](googleapis/java-shared-dependencies@84b81e9))
* update google.common-protos.version to v2.9.1 ([#724](googleapis/java-shared-dependencies#724)) ([62cd59a](googleapis/java-shared-dependencies@62cd59a))
* update google.core.version to v2.8.1 ([#725](googleapis/java-shared-dependencies#725)) ([d47af56](googleapis/java-shared-dependencies@d47af56))
* update google.core.version to v2.8.3 ([#760](googleapis/java-shared-dependencies#760)) ([33e38dc](googleapis/java-shared-dependencies@33e38dc))
* update google.core.version to v2.8.4 ([#762](googleapis/java-shared-dependencies#762)) ([5410450](googleapis/java-shared-dependencies@5410450))
* update google.core.version to v2.8.5 ([#764](googleapis/java-shared-dependencies#764)) ([4bc8c75](googleapis/java-shared-dependencies@4bc8c75))
* update iam.version to v1.5.0 ([#732](googleapis/java-shared-dependencies#732)) ([3e64541](googleapis/java-shared-dependencies@3e64541))
* update iam.version to v1.5.1 ([#737](googleapis/java-shared-dependencies#737)) ([5a85115](googleapis/java-shared-dependencies@5a85115))
* update iam.version to v1.5.2 ([#743](googleapis/java-shared-dependencies#743)) ([294ea85](googleapis/java-shared-dependencies@294ea85))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
suztomo pushed a commit that referenced this pull request Mar 21, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants