feat!: [Spanner] Upgrade to V2 #7193
Conversation
ajupazhamayil
force-pushed
the
spanner-v2
branch
2 times, most recently
from
843be5e
to
55a81e9
Compare
ajupazhamayil
force-pushed
the
spanner-v2
branch
from
55a81e9
to
37076aa
Compare
We did not create any design document for these changes. We are following a V2 parent doc drafted by @saranshdhingra. Basically we are trying to remove usage of Please let me know if you would like me to create a doc for this purpose. |
ajupazhamayil
changed the title
ajupazhamayil
changed the title
bshaffer
changed the title
ajupazhamayil
changed the title
Spanner: upgrade to V2 (Part 2/2)
ajupazhamayil
changed the title
|
The "Prefer Lowest" tests are failing because you've added a new |
Great, the test is passing now.. |
| * | ||
| * @return array | ||
| */ | ||
| private function getLROResponseMappers() |
There was a problem hiding this comment.
Removing duplicate data in the handwritten layer that's already being generated and handled by GAPIC is the whole reason we are making the V2 changes, so having all this metadata defined in an array like this is not acceptable.
We can consume the generated data in the GAPIC layer, or use the GAPIC clients directly.
| * @param $clientName The class name of the client. | ||
| * @param $clientObject The client object to be added. | ||
| */ | ||
| public function addClientObject(string $clientName, mixed $clientObject) |
There was a problem hiding this comment.
Having public getters and setters for the client objects is hacky and could have unexpected results. This points to a flaw in the design.
| array $info = [], | ||
| $databaseRole = '' | ||
| string $databaseRole = '', | ||
| array $config = [] |
There was a problem hiding this comment.
I do not like extending our already bloated constructor with another parameter, especially one as generic as $config.
If anything , we should name this $options and combine it with all the other optional parameters.
Spanner V2:
Part 1:
RequestHandler[Operationclass]RequestHandler[Databaseclass]RequestHandler[Sessionclass]RequestHandler[CacheSessionPoolclass]RequestHandlerin Core'sLongRunningOperation]Part 2:
RequestHandler[Backupclass]RequestHandler[Instanceclass]RequestHandler[InstanceConfigurationclass]RequestHandler[SpannerClient]Tests:
Related work:
OperationsClient[GAPIC generator change] [See @todo in Spanner/src/Admin/Database/V1/Client/DatabaseAdminClient.php] : This is replaced with the migration status changes.BREAKING_CHANGE_REASON=RC-1 for the version 2 of the Spanner library.