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: declare depenencies of API surfaces as api #1535

Merged
merged 4 commits into from Oct 18, 2021

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Oct 16, 2021

Fixes: #1534.

https://docs.gradle.org/current/userguide/publishing_maven.html says:

Gradle will also use the versions resolved on the runtimeClasspath for dependencies declared in implementation, which are mapped to the runtime scope of Maven

If a class is exposed as API surface, then the dependency should be declared using 'api'.

google-auth-library-oauth2-http

For the classes in com.google.auth.oauth2 (com.google.auth.oauth2.QuotaProjectIdProvider, com.google.auth.oauth2.ComputeEngineCredentials, com.google.auth.oauth2.GoogleCredentials, com.google.auth.oauth2.ServiceAccountCredentials, com.google.auth.oauth2.ServiceAccountJwtAccessCredentials, com.google.auth.oauth2.QuotaProjectIdProvider), I didn't find any of them used as API surface. Therefore "implementation" makes sense.

google-auth-library-credentials

I found com.google.auth.Credentials class in google-auth-library-credentials is exposed in gax. (This artifact is not declared in gax's build.gradle.)

Other artifacts

org_threeten_threetenbp

The artifact defines org.threeten.bp.Duration.

gax exposes it in its public API

public abstract Duration getDelayThreshold();

gax-grpc uses it in its public API

gax-httpjson uses it in its public API

com_google_api_api_common

gax uses com.google.api.core.ApiFuture in its public API

public ApiFuture<Void> pushCurrentBatch() {
.

gax-grpc uses it

public static ApiFuture<ListOperationsPagedResponse> createAsync(

gax-httpjson uses it

<ResponseT, RequestT> ApiFuture<ResponseT> issueFutureUnaryCall(

io_opencensus_opencensus_api

It doesn't appear in the API surfaces of gax, gax-grpc, and gax-httpjson.

grpc-api

gax-grpc uses it in public API

grpc-auth, grpc-protobuf, grpc-netty-shaded, grpc-alts, grpc-stub

They are not in public API.

guava

gax-grpc uses com.google.common.collect.ImmutableList in public API

public ImmutableList<UnaryCallSettings.Builder<?, ?>> unaryMethodSettingsBuilders() {

gax-httpjson uses it in public API

public ImmutableList<UnaryCallSettings.Builder<?, ?>> unaryMethodSettingsBuilders() {

com_google_api_grpc_proto_google_common_protos

gax-grpc uses com.google.longrunning.Operation in public API

GrpcCallSettings<RequestT, Operation> grpcCallSettings,

gax-httpjson uses com.google.longrunning.ListOperationsRequest in public API

public UnaryCallable<ListOperationsRequest, ListOperationsPagedResponse>

com_google_protobuf

gax-httpjson uses com.google.protobuf.TypeRegistry in public API

public ResponseT parse(InputStream httpResponseBody, TypeRegistry registry) {

com_google_protobuf_java_util

It's not used as public API.

com_google_code_gson_gson

gax-httpjson uses com.google.gson.JsonElement in public API

com_google_http_client_google_http_client

gax-httpjson uses com.google.api.client.http.HttpTransport in public API

public Builder setHttpTransport(HttpTransport httpTransport) {

com_google_http_client_google_http_client_gson

This is not used in public API.

@suztomo suztomo requested review from as code owners Oct 16, 2021
@google-cla google-cla bot added the cla: yes label Oct 16, 2021
gax-grpc/build.gradle Outdated Show resolved Hide resolved
archivesBaseName = "gax-httpjson"

// TODO: Populate this from dependencies.properties version property (for proper Gradle-Bazel sync)
project.version = "0.91.1-SNAPSHOT" // {x-version-update:gax-httpjson:current}

dependencies {
implementation( project(':gax'),
libraries['maven.com_google_protobuf'],
api ( project(':gax'),
Copy link
Member Author

@suztomo suztomo Oct 16, 2021

Choose a reason for hiding this comment

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

At least gax's StatusCode is exposed as API surface.

public Set<StatusCode.Code> getRetryableCodes() {

archivesBaseName = "gax-grpc"

// TODO: Populate this from dependencies.properties version property (for proper Gradle-Bazel sync)
project.version = "2.6.1-SNAPSHOT" // {x-version-update:gax-grpc:current}

dependencies {
implementation( project(':gax'),
libraries['maven.io_grpc_grpc_stub'],
api (project(':gax'),
Copy link
Member Author

@suztomo suztomo Oct 16, 2021

Choose a reason for hiding this comment

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

At least gax's StatusCode is exposed as API surface.

public Set<StatusCode.Code> getRetryableCodes() {

implementation( project(':gax'),
libraries['maven.io_grpc_grpc_stub'],
api (project(':gax'),
libraries['maven.com_google_auth_google_auth_library_credentials'])
Copy link
Member Author

@suztomo suztomo Oct 16, 2021

Choose a reason for hiding this comment

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

com.google.auth.Credentials is part of API surface

public GrpcCallContext withCredentials(Credentials newCredentials) {

implementation( project(':gax'),
libraries['maven.com_google_protobuf'],
api ( project(':gax'),
libraries['maven.com_google_auth_google_auth_library_credentials'])
Copy link
Member Author

@suztomo suztomo Oct 16, 2021

Choose a reason for hiding this comment

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

com.google.auth.Credentials is part of API surface.

gax/build.gradle Outdated
archivesBaseName = "gax"

// TODO: Populate this from dependencies.properties version property (for proper Gradle-Bazel sync)
project.version = "2.6.1-SNAPSHOT" // {x-version-update:gax:current}

dependencies {
api libraries['maven.com_google_auth_google_auth_library_credentials']
Copy link
Member Author

@suztomo suztomo Oct 16, 2021

Choose a reason for hiding this comment

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

com.google.auth.Credentials is part of API surface #1534 (comment)

Screen Shot 2021-10-15 at 10 53 38 PM

Adding more artifacts to be declared as 'api' because they appear
in public API surface. See the comment of the following issue
for the analysis:
googleapis#1534
@suztomo suztomo changed the title deps: upgrading auth library and declare it as api deps: declare depenencies of API surfaces as api Oct 18, 2021
@suztomo suztomo changed the title deps: declare depenencies of API surfaces as api fix: declare depenencies of API surfaces as api Oct 18, 2021
@suztomo suztomo requested a review from chanseokoh Oct 18, 2021
Copy link
Member Author

@suztomo suztomo left a comment

@chanseokoh PTAL.

gax-grpc/build.gradle Outdated Show resolved Hide resolved
build.gradle Show resolved Hide resolved
@suztomo suztomo merged commit 725414f into googleapis:main Oct 18, 2021
5 checks passed
suztomo added a commit to suztomo/gax-java that referenced this issue Oct 18, 2021
* deps: declaring the latest auth library version

* fix: declare API surface as api configuration

Fix googleapis#1534

* fix: declare API surface's dependencies as 'api'

Adding more artifacts to be declared as 'api' because they appear
in public API surface. See the comment of the following issue
for the analysis:
googleapis#1534

* refactor: move java-library declaration to top-level build.gradle
suztomo added a commit that referenced this issue Oct 18, 2021
* deps: declaring the latest auth library version

* fix: declare API surface as api configuration

Fix #1534

* fix: declare API surface's dependencies as 'api'

Adding more artifacts to be declared as 'api' because they appear
in public API surface. See the comment of the following issue
for the analysis:
#1534

* refactor: move java-library declaration to top-level build.gradle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants