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: Mark getDefaultEndpoint() with @ObsoleteApi #2347

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented Jan 5, 2024

Client libraries no longer always use the default endpoint. The resolved endpoint will be determined by the EndpointContext and used by the client.

@lqiu96 lqiu96 requested a review from a team as a code owner January 5, 2024 17:33
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Jan 5, 2024
AnnotationNode.builder()
.setType(FIXED_TYPESTORE.get(ObsoleteApi.class.getSimpleName()))
.setDescription(
"See https://github.com/googleapis/sdk-platform-java/issues/2346 for more information")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we provide a brief information in the description first? So that customers don't have to click through the link when they hover over on the method? Something like "There is no concept of a default endpoint anymore with the introduction of universeDomain, please use ... instead".

In addition, maybe we can add the annotation only until the alternative getResolvedEndpoint() is introduced later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll can add a description and hold off on adding the annotation until a future date.

until the alternative getResolvedEndpoint() is introduced

I don't think getResolvedEndpoint() is going to be a 100% true alternative to getDefaultEndpoint(). Based on the previous configurations (prior to EndpointContext), the default endpoint may not be the resolved endpoint used by the client. We're no longer supporting a concept of a default endpoint so there really should not be an alternative to getDefaultEndpoint().

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a fan of having github links in comments, I think they should be used only if we have no other way. They could become invalid for a lot of reasons, repo access changes, Github URL format change etc., or maybe the source of truth is moved elsewhere. Can we add more info in the annotation description itself?

@lqiu96 lqiu96 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 5, 2024
@lqiu96
Copy link
Contributor Author

lqiu96 commented Jan 5, 2024

Removing the getDefaultEndpoint() from being set in this PR: #2348

@lqiu96 lqiu96 force-pushed the mark-getDefaultEndpoint-obsoleteApi branch from 7cf57ca to 9ea0253 Compare July 11, 2024 21:40
@lqiu96 lqiu96 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 11, 2024
Copy link
Contributor

@mpeddada1 mpeddada1 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 will let @blakeli0 give the final approval.

Copy link

sonarcloud bot commented Jul 12, 2024

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
30.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Jul 12, 2024

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
50.0% Duplication on New Code

See analysis details on SonarCloud

@lqiu96 lqiu96 merged commit e46648f into main Jul 12, 2024
33 of 34 checks passed
@lqiu96 lqiu96 deleted the mark-getDefaultEndpoint-obsoleteApi branch July 12, 2024 20:28
lqiu96 pushed a commit that referenced this pull request Jul 25, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>2.43.0</summary>

##
[2.43.0](v2.42.0...v2.43.0)
(2024-07-25)


### Features

* add `transport` option to `generation_config.yaml`
([#3052](#3052))
([3b1a915](3b1a915))
* get released version from versions.txt to render `README.md`
([#3007](#3007))
([99bb2b3](99bb2b3))
* Introduce java.time to Gax-Java
([#1872](#1872))
([308aeaf](308aeaf))
* Mark `getDefaultEndpoint()` with @ObsoleteApi
([#2347](#2347))
([e46648f](e46648f))
* parse `BUILD.bzel` to determine whether a commit that only changed
`BUILD.bazel` is a qualified commit
([#2937](#2937))
([502f801](502f801))


### Bug Fixes

* Fix:
([d996c2d](d996c2d))
* `BaseApiTracer` to noop on attemptFailed via overloaded method call
([#3016](#3016))
([2fc938a](2fc938a))
* Generator to skip generation for empty services.
([#3051](#3051))
([ff2c485](ff2c485))
* restore hermetic build image publication
([#2952](#2952))
([97a6d67](97a6d67))


### Dependencies

* update dependency com.fasterxml.jackson:jackson-bom to v2.17.2
([#3028](#3028))
([d16f9d1](d16f9d1))
* update dependency
com.google.cloud.opentelemetry:detector-resources-support to v0.30.0
([#2975](#2975))
([b3ec93f](b3ec93f))
* update dependency
com.google.cloud.opentelemetry:detector-resources-support to v0.31.0
([#3044](#3044))
([6bd07dc](6bd07dc))
* update dependency com.google.errorprone:error_prone_annotations to
v2.29.2
([#3058](#3058))
([8ea0868](8ea0868))
* update dependency com.google.errorprone:error_prone_annotations to
v2.29.2
([#3059](#3059))
([81b23dc](81b23dc))
* update dependency com.google.guava:guava to v33.2.1-jre
([#3027](#3027))
([12ee456](12ee456))
* update dependency commons-codec:commons-codec to v1.17.1
([#3049](#3049))
([58d94b7](58d94b7))
* update dependency dev.cel:cel to v0.6.0
([#3050](#3050))
([bc332d9](bc332d9))
* update dependency net.bytebuddy:byte-buddy to v1.14.18
([#3029](#3029))
([8799cf6](8799cf6))
* update dependency org.apache.commons:commons-lang3 to v3.15.0
([#3060](#3060))
([2538334](2538334))
* update dependency org.checkerframework:checker-qual to v3.45.0
([#2988](#2988))
([4edd216](4edd216))
* update google api dependencies
([#2951](#2951))
([c16f6c9](c16f6c9))
* update google auth library dependencies to v1.24.0
([#3039](#3039))
([98b5bd7](98b5bd7))
* update googleapis/java-cloud-bom digest to 47c5dbc
([#2974](#2974))
([57623f0](57623f0))
* update grpc dependencies to v1.65.1
([#3061](#3061))
([27497e2](27497e2))
* update junit5 monorepo to v5.10.3
([#2963](#2963))
([bc55fe1](bc55fe1))
* update netty dependencies to v4.1.112.final
([#3057](#3057))
([5af127b](5af127b))
* update opentelemetry-java monorepo to v1.40.0
([#3035](#3035))
([5c31c42](5c31c42))
* Use Gapic-Showcase v0.35.1
([#3018](#3018))
([43773f0](43773f0))


### Documentation

* add support option to 'new issue' choices
([#3055](#3055))
([6a2a17d](6a2a17d))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants