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: remove extends ApiMessage from HttpJsonStubCallableFactory definition #1426

Merged
merged 1 commit into from Jul 13, 2021

Conversation

vam-google
Copy link
Contributor

@vam-google vam-google commented Jul 6, 2021

This is needed to match DIREGAPIC architecture, which does not rely on ApiMessage but on the proto stubs instead. This is also a prerequisite to making LRO a non-breaking change post GCE client GA.

The full set (without tests) of anticipated changes to support DIREGAPIC LRO can be found in 8588755 (a separate branch now). The rest of the changes are additive, that is why they can wait.

…efinition

This is needed to match DIREGAPIC architecture, which does not rely on ApiMessage but on the proto stubs instead.
@vam-google vam-google requested review from as code owners Jul 6, 2021
@google-cla google-cla bot added the cla: yes label Jul 6, 2021
@vam-google vam-google requested review from vchudnov-g and miraleung Jul 6, 2021
Copy link
Contributor

@miraleung miraleung left a comment

Wouldn't this be an API-breaking change for public dev users who use this library?

@vam-google
Copy link
Contributor Author

@vam-google vam-google commented Jul 12, 2021

@miraleung I don't think so. I'm not even sure if it is breaking change on binary level (I guess it is no, as it passes the prebuilds). This makes the generic accept more stuff than before, so it expands the scope, not narrows it down, so everybody who was using the stuff with the more narrow scopse shoudl be able to use it. I actually tested it with compute client, and it is just fine (both new and old gaxes work on the same library).

@vam-google vam-google requested a review from miraleung Jul 12, 2021
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.

None yet

2 participants