Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

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 a team as code owners July 6, 2021 23:53
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 6, 2021
Copy link
Contributor

@miraleung miraleung left a comment

Choose a reason for hiding this comment

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

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

@vam-google
Copy link
Contributor Author

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants