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.

Sign up for GitHub

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

Jump to bottom

Some dependencies in java-alloydb/google-cloud-alloydb module seem to have wrong dependency scopes. #10037

Closed
HappyHacker123 opened this issue Nov 3, 2023 · 1 comment · Fixed by googleapis/sdk-platform-java#3072
Closed

Some dependencies in java-alloydb/google-cloud-alloydb module seem to have wrong dependency scopes. #10037

HappyHacker123 opened this issue Nov 3, 2023 · 1 comment · Fixed by googleapis/sdk-platform-java#3072
Assignees
@diegomarquezp
Labels
api: alloydb Issues related to the AlloyDB API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@HappyHacker123
Copy link

Description

Dependency "com.google.api.grpc:grpc-google-common-protos" and "com.google.api.grpc:grpc-google-iam-v1" in the java-alloydb/google-cloud-alloydb module is set to the default dependency scope "compile". However, it seems that they are only used for tests.

    <dependency>
      <groupId>com.google.api.grpc</groupId>
      <artifactId>grpc-google-common-protos</artifactId>
    </dependency>
    <dependency>
      <groupId>com.google.api.grpc</groupId>
      <artifactId>grpc-google-iam-v1</artifactId>
    </dependency>

Possible Outcome

This brings redundant compile time dependency not only for google-cloud-alloydb, but also for downstream projects using it. Changing dependency scope to test can help maintaining a correct and simpler dependency tree.

Solution

I modified the scopes to "test", proceeded to recompile the module, and then ran the tests. As a result of these modifications, the module compiled without issues, and all the tests were successfully passed.
@HappyHacker123 HappyHacker123 mentioned this issue Nov 3, 2023
Open
@HappyHacker123
Copy link
Author

HappyHacker123 commented Nov 3, 2023 •
edited
Loading

I raise a pr #10038 about this issue. Could you help me review this issue @suztomo @meltsufin? many thanks :)

@blakeli0 blakeli0 added priority: p2 Moderately-important priority. Fix may not be included in next release. api: alloydb Issues related to the AlloyDB API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 3, 2023
@zhumin8 zhumin8 added priority: p3 Desirable enhancement or fix. May not be included in next release. status: blocked Resolving the issue is dependent on other work. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Apr 23, 2024
@blakeli0 blakeli0 added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p3 Desirable enhancement or fix. May not be included in next release. status: blocked Resolving the issue is dependent on other work. labels Jul 10, 2024
@diegomarquezp diegomarquezp self-assigned this Jul 18, 2024
@diegomarquezp diegomarquezp mentioned this issue Jul 26, 2024
Merged
ldetmer pushed a commit to googleapis/sdk-platform-java that referenced this issue Sep 17, 2024
@diegomarquezp @ldetmer
chore: generate poms with grpc dependencies as test scoped (#3072)
b196489 
Fixes googleapis/google-cloud-java#10037 ☕ 

### Overview

This will modify the behavior of library generation at the
postprocessing stage. The main change is that generated _cloud_ poms
(i.e. `google-cloud-${service}/pom.xml`) will now have the dependencies
`proto-google-common-protos` and `grpc-google-iam-v1` declared as
`<scope>test</scope>` if the libraries belong to a monorepo.

### Integration test

It will be failing until we update the test branch. Once this approach
is validated, I will proceed with updating the branch.

### Approach
#### For new libraries
Via template in `owlbot/teplates/poms/cloud_pom.xml.j2`. New libraries
will have these two dependencies declared as test scope if they are part
of a monorepo.

#### For existing libraries
The script `fix_poms.py` will now modify the existing poms by setting
the two dependencies as test-scoped, only if they are part of a
monorepo.

### Confirmation that it works
* java-bigquerystorage: no effects since these dependencies are [not
found in the
pom](https://github.com/search?q=repo%3Agoogleapis%2Fjava-bigquerystorage%20path%3Agoogle-cloud-bigquerystorage%2Fpom.xml%20grpc-google&type=code)
* java-datastore: [no relevant
changes](https://github.com/googleapis/java-datastore/pull/1530/files)
since the dependencies are [not found in the
pom](https://github.com/googleapis/java-datastore/blob/main/google-cloud-datastore/pom.xml)
* java-firestore: [no relevant
changes](googleapis/java-firestore#1764) since
the one targeted dependency is [already declared as test
scoped](https://github.com/googleapis/java-firestore/blob/main/google-cloud-firestore/pom.xml#L160-L164)
* java-logging: [no relevant
changes](googleapis/java-logging#1660)] since
the dependencies are [not declared in the
pom](https://github.com/googleapis/java-logging/blob/main/google-cloud-logging/pom.xml)
* java-pubsub: no changes. Relevant dependency [already declared as test
scoped](https://github.com/googleapis/java-pubsub/blob/2b15b0a3f43c9ccef663fedf0398375f58fd9183/google-cloud-pubsub/pom.xml#L135-L139)
* java-pubsublite: [no relevant
changes](googleapis/java-pubsublite#1692) since
there are [no relevant dependencies declared in the
pom](https://github.com/googleapis/java-pubsublite/blob/main/google-cloud-pubsublite/pom.xml)
 * java-spanner: ignored - dependency kept as is
 * java-storage: ignored - kept as is
* **google-cloud-java: [91 poms
affected](https://github.com/googleapis/google-cloud-java/pull/11031/files)
by converting dependencies to test-scoped (PR only contains pom changes
for simplicity + outdated googleapis_committish)**
* We had a special situation with `java-dataproc` because it references
`google-iam-v1` via `grpc-google-iam-v1`. However this is incorrect
because what this library really needs is the proto classes. The fix for
this was to treat this library as any other library and add
`proto-google-iam-v1` as a dependency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@diegomarquezp diegomarquezp

Labels
api: alloydb Issues related to the AlloyDB API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: generate poms with grpc dependencies as test scoped googleapis/sdk-platform-java
4 participants
@zhumin8 @blakeli0 @diegomarquezp @HappyHacker123