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(Spanner): Support Emulator via env in Spanner Admin Client #687

Merged
merged 19 commits into from
Feb 28, 2024

Conversation

ajupazhamayil
Copy link
Contributor

@ajupazhamayil ajupazhamayil commented Feb 22, 2024

This PR introduces emulator support in the Spanner Admin Client via an environment variable. These changes will simplify future similar requirements.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Feb 22, 2024
@ajupazhamayil ajupazhamayil force-pushed the generator_spanner_emulator_support branch from f09e36e to b04b6da Compare February 22, 2024 09:41
@vishwarajanand
Copy link
Contributor

vishwarajanand commented Feb 22, 2024

Test failures look unrelated, potential fix: #688

@ajupazhamayil
Copy link
Contributor Author

Referred: #688

src/Ast/AST.php Outdated Show resolved Hide resolved
src/Generation/EmulatorSupportGenerator.php Outdated Show resolved Hide resolved
tests/Unit/ProtoTests/Basic/out/src/Client/BasicClient.php Outdated Show resolved Hide resolved
@ajupazhamayil
Copy link
Contributor Author

@bshaffer Thank you for the quick review, I have resolved your comments.

@ajupazhamayil
Copy link
Contributor Author

I have checked the following test to understand it is in fact working for the Spanner Admin Client.

$diff tests/Integration/goldens/spanner/src/V1/Client/DatabaseAdminClient.php ../google-cloud-php/Spanner/src/Admin/Database/V1/Client/DatabaseAdminClient.php

<  * Copyright 2024 Google LLC
---
>  * Copyright 2023 Google LLC
38d37
< use Google\Cloud\Core\InsecureCredentialsWrapper;
71d69
< use Grpc\ChannelCredentials;
375c373
<         $clientOptions = $this->buildClientOptions($options + $this->getDefaultEmulatorConfig());
---
>         $clientOptions = $this->buildClientOptions($options);
1037,1062d1034
<     }
< 
<     /** Configure the gapic configuration to use a service emulator. */
<     private function getDefaultEmulatorConfig(): array
<     {
<         $emulatorHost = getenv('SPANNER_EMULATOR_HOST');
<         if (empty($emulatorHost)) {
<             return [];
<         }
< 
<         if ($scheme = parse_url($emulatorHost, PHP_URL_SCHEME)) {
<             $search = $scheme . '://';
<             $emulatorHost = str_replace($search, '', $emulatorHost);
<         }
< 
<         return [
<             'apiEndpoint' => $emulatorHost,
<             'transportConfig' => [
<                 'grpc' => [
<                     'stubOpts' => [
<                         'credentials' => ChannelCredentials::createInsecure(),
<                     ],
<                 ],
<             ],
<             'credentials' => new InsecureCredentialsWrapper(),
<         ];

@ajupazhamayil ajupazhamayil marked this pull request as ready for review February 23, 2024 20:16
@ajupazhamayil ajupazhamayil requested review from a team as code owners February 23, 2024 20:16
Copy link

snippet-bot bot commented Feb 23, 2024

Here is the summary of changes.

You are about to add 20 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

Copy link
Contributor

@bshaffer bshaffer left a comment

Choose a reason for hiding this comment

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

One minor suggestion, otherwise this is looking great. Would also love some eyeballs from @noahdietz

@ajupazhamayil ajupazhamayil added the release blocking Required feature/issue must be fixed prior to next release. label Feb 27, 2024
Copy link
Collaborator

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

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

One comment on the integration test but otherwise, LGTM

tests/Integration/BUILD.bazel Outdated Show resolved Hide resolved
@bshaffer bshaffer merged commit 5409e38 into main Feb 28, 2024
5 checks passed
@bshaffer bshaffer deleted the generator_spanner_emulator_support branch February 28, 2024 14:54
@bshaffer
Copy link
Contributor

@ajupazhamayil nit, but we only use tags like feat(Spanner) in google-cloud-php. It doesn't make sense in the generator here because this generator applies to all GAPIC APIs, and shouldn't be thought of as applying to a specific one. Even though this feature is only going to roll out to Spanner, it's a generic feature which could apply to any package/API.

@ajupazhamayil
Copy link
Contributor Author

@ajupazhamayil nit, but we only use tags like feat(Spanner) in google-cloud-php. It doesn't make sense in the generator here because this generator applies to all GAPIC APIs, and shouldn't be thought of as applying to a specific one. Even though this feature is only going to roll out to Spanner, it's a generic feature which could apply to any package/API.

Sure @bshaffer, Will take care of this from next time onwards. Thank you for the information.

@ajupazhamayil ajupazhamayil removed the release blocking Required feature/issue must be fixed prior to next release. label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants