feat: GA Gax HttpJson by lqiu96 · Pull Request #1264 · googleapis/sdk-platform-java

lqiu96 · 2023-01-20T18:47:59Z

We will begin the process of trying to GA Gax's HttpJson module. @BetaApi will slowly be removed where it makes sense as we have observed stability in this module.

sonarqubecloud · 2023-01-25T22:51:11Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

lqiu96 · 2023-06-08T20:11:46Z

gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java

    return streamIdleTimeout;
  }

-  @BetaApi("The surface for extra headers is not stable yet and may change in the future.")


Issue to better support extraHeaders: https://github.com/googleapis/sdk-platform-java/issues/1752

Will be done in the future.

If this BetaApi annotation is removed in gax-httpjson, should it also be removed in gax and gax-grpc?

If the base class/interface is still BetaApi, we don't have to remove them, I would only remove BetaApi for httpjson specific classes.

If the base class/interface is still BetaApi, we don't have to remove them, I would only remove BetaApi for httpjson specific classes.

Just to clarify what you mean by httpjson specific classes. Do you mean:

Remove annotation for classes/ methods that exist ONLY in the httpjson module and no other gax module (withExtraHeaders would keep the BetaApi annotation)

Remove annotation only from the httpjson module (i.e. Remove in httpjson but keep in gax and gax-grpc)

1 yes, 2 no. In addition, if a class does not extend from classes in other modules, we should remove BetaApi from it unless there is a reason not to(e.g. maybe the feature in grpc is also still in Beta).

Gotcha. I'll check the files again and update.

lqiu96 · 2023-06-08T20:22:44Z

@blakeli0 @suztomo @ddixit14 I'm looking to begin the process of GAing Gax-HttpJson. I want to see your opinions of what version I should set for this module's GA: v1.0.0 or v2.30.0 (I believe v2.30.0 should be the next version of gax & gax-grpc).

Immediately bumping to v2.30.0 would keep all the gax versions in sync, but may be confusing to customers who see the bump from v0.114.1 -> v2.30.0.

suztomo · 2023-06-09T00:21:30Z

I'm for "v2.30.0 (should match the next version of gax & gax-grpc)". The consistent version gives clarity in error messages when a dependency conflict happens.

lqiu96 · 2023-06-09T17:41:09Z

Things left with BetaApi annotation:

HttpJsonCallContext: withExtraHeaders and getExtraHeaders - HttpJsonCallContext extends from ApiCallContext and the methods override from there. The parent methods still have BetaApi annotation

Note:

HttpJsonOperationSnapshotCallable extends from UnaryCallable, but UnaryCallable has no BetaApi annotation (also oddly enough has both BetaApi and Internal annotations)
HttpJsonStatusCode extends from StatusCode, but StatusCode has no BetaApi annotation

Weird behaviors:

InstantiatingHttpJsonChannelProvider extends from TransportChannelProvider. TransportChannelProvider has two BetaApi annotations in needsCredentials and withCredentials, but InstantiatingHttpJsonChannelProvider's overriden methods don't have BetaApi annotations

lqiu96 · 2023-06-09T19:01:00Z

I'm for "v2.30.0 (should match the next version of gax & gax-grpc)". The consistent version gives clarity in error messages when a dependency conflict happens.

I've updated the current for gax-httpjson to be 2.29.1-SNAPSHOT and I believe with this feat: PR, it'll mark it as 2.30.0. If anyone has any concerns, let me know.

lqiu96 · 2023-06-20T15:19:31Z

@suztomo @blakeli0 @ddixit14 Would you all be able to review this PR? If there are any concerns, I can target a different cycle to unblock this release train.

blakeli0

Thanks Lawrence! LGTM.

suztomo · 2023-06-20T18:47:29Z

coverage-report/pom.xml

                <groupId>com.google.api</groupId>
                <artifactId>gax-httpjson</artifactId>
-                <version>0.114.1-SNAPSHOT</version> <!-- {x-version-update:gax-httpjson:current} -->
+                <version>2.29.1-SNAPSHOT</version> <!-- {x-version-update:gax-httpjson:current} -->


Can you remove {x-version-update:gax-httpjson:current} altogether in favor of version.gax

Ah, good idea! Updated in this file as well as the gax-bom. I noticed that we still have gax-grpc. Shall I remove this that reference as well (or maybe in a different PR)?

suztomo · 2023-06-20T19:12:01Z

HttpJsonCallContext's withExtraHeaders is marked as BetaApi. Is this intentional (I'm not saying you should remove it).

~/sdk-platform-java/gax-java/gax-httpjson $ grep -ilr -A 2 '@BetaApi' .                    git[branch:main-gax_httpjson_GA]
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java
suztomo@suztomo-macbookpro.roam.internal ~/sdk-platform-java/gax-java/gax-httpjson
~/sdk-platform-java/gax-java/gax-httpjson $ grep -ir -A 2 '@BetaApi' .                     git[branch:main-gax_httpjson_GA]
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java:  @BetaApi("The surface for extra headers is not stable yet and may change in the future.")
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  @Override
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  public ApiCallContext withExtraHeaders(Map<String, List<String>> extraHeaders) {
--
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java:  @BetaApi("The surface for extra headers is not stable yet and may change in the future.")
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  @Override
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  public Map<String, List<String>> getExtraHeaders() {

lqiu96 · 2023-06-20T19:17:57Z

HttpJsonCallContext's withExtraHeaders is marked as BetaApi. Is this intentional (I'm not saying you should remove it).

~/sdk-platform-java/gax-java/gax-httpjson $ grep -ilr -A 2 '@BetaApi' .                    git[branch:main-gax_httpjson_GA]
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java
suztomo@suztomo-macbookpro.roam.internal ~/sdk-platform-java/gax-java/gax-httpjson
~/sdk-platform-java/gax-java/gax-httpjson $ grep -ir -A 2 '@BetaApi' .                     git[branch:main-gax_httpjson_GA]
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java:  @BetaApi("The surface for extra headers is not stable yet and may change in the future.")
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  @Override
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  public ApiCallContext withExtraHeaders(Map<String, List<String>> extraHeaders) {
--
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java:  @BetaApi("The surface for extra headers is not stable yet and may change in the future.")
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  @Override
./src/main/java/com/google/api/gax/httpjson/HttpJsonCallContext.java-  public Map<String, List<String>> getExtraHeaders() {

This is intentional. The context is here: #1264 (comment).

sonarqubecloud · 2023-06-20T19:20:36Z

[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

sonarqubecloud · 2023-06-20T19:24:51Z

[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

sonarqubecloud · 2023-06-20T19:29:13Z

[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

lqiu96 added 7 commits January 18, 2023 11:39

feat: Remove @BetaApi for certain httpjson classes

0a2848b

Merge branch 'main' into main-gax_httpjson_GA

9ea766d

chore: Fix formatting issues

e53ad2f

chore: Fix formatting issues

c038911

chore: Remove a few more @BetaApi annotations

3b7b024

chore: Fix format issues

c65e9f4

Merge branch 'main' into main-gax_httpjson_GA

2e76762

chore: Resolve merge conflicts

e981d9e

product-auto-label bot added the size: m Pull request size is medium. label Jun 8, 2023

lqiu96 commented Jun 8, 2023

View reviewed changes

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 8, 2023

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 8, 2023

lqiu96 marked this pull request as ready for review June 8, 2023 20:12

lqiu96 requested a review from a team June 8, 2023 20:12

chore: Revert BetaApi annotation for extraHeaders in gax

d034e0c

lqiu96 requested review from blakeli0, ddixit14 and suztomo June 8, 2023 20:18

chore: Update BetaApi annotations for subclasses

0da88ed

lqiu96 added 2 commits June 9, 2023 13:43

chore: Update release please version

a9bee88

chore: Update all dependencies to new gax-httpjson version

865822f

blakeli0 approved these changes Jun 20, 2023

View reviewed changes

suztomo reviewed Jun 20, 2023

View reviewed changes

chore: Update x-version-update tag to be gax-version

cebebe8

suztomo approved these changes Jun 20, 2023

View reviewed changes

lqiu96 merged commit 9f15fea into main Jun 20, 2023

lqiu96 deleted the main-gax_httpjson_GA branch June 20, 2023 22:02

release-please bot mentioned this pull request Jun 20, 2023

chore(main): release 2.22.0 #1761

Merged

zhumin8 mentioned this pull request Jun 21, 2023

chore: Update dependencies.properties to use gax's version #1792

Merged

Conversation

lqiu96 commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lqiu96 commented Jun 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suztomo commented Jun 9, 2023

Uh oh!

lqiu96 commented Jun 9, 2023

Uh oh!

lqiu96 commented Jun 9, 2023

Uh oh!

lqiu96 commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blakeli0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suztomo commented Jun 20, 2023

Uh oh!

lqiu96 commented Jun 20, 2023

Uh oh!

sonarqubecloud bot commented Jun 20, 2023

Uh oh!

sonarqubecloud bot commented Jun 20, 2023

Uh oh!

sonarqubecloud bot commented Jun 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lqiu96 commented Jan 20, 2023 •

edited

Loading

lqiu96 commented Jun 8, 2023 •

edited

Loading

lqiu96 commented Jun 20, 2023 •

edited

Loading