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.

Sign up for GitHub

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

Jump to bottom

Refresh gapics #679

Merged

dwsupplee merged 17 commits into googleapis:master from michaelbausor:refresh-gapic-format-retry

Oct 9, 2017

Contributor

michaelbausor commented Sep 19, 2017

Things to note:

The updateSubscription Pubsub method has been removed from GAPIC and manual. This method is currently under whitelist - I don't think it should have been added in either place in the first place. Please verify that I am not going crazy, and that this is indeed under whitelist and needs to be removed, and that I haven't just forgotten why it was there. If we want to add it back with appropriate whitelist documentation, we can do it along with updateTopic and updateSnapshot, which are also under whitelist.
Manual edits are present to support the GAX/GAPIC updates, mostly in Spanner for the format/parse methods
Tests won't pass until we release new versions of GAX and proto-client. If you want to test locally, you should be able to pull the branch and set proto-client to 0.23 and gax to dev-master.

It would be great to get any feedback early-ish tomorrow so I can make sure it gets fixed ASAP, and you will be able to merge this in my absence

michaelbausor added 9 commits

September 18, 2017 15:20


          Spanner updates

f4c5235


          Update GAPICs

1fee028


          Test updates

6a0cfc6


          Remove updateSubscription

76c0ec7


          Add SpannerClient partial veneer back

131c42d


          Refresh GAPICs

8147b06


          Update snippets, remove updateSubscription method

0e47dd4


          Update gax and proto-client versions

4d34a26


          Manually edit resources files

7624c08

googlebot added the cla: yes label

dwsupplee added the gapic label

Contributor

dwsupplee commented Sep 19, 2017

It certainly appears you're correct that updateSubscription should have had a whitelist notification added to it as it looks like it was added when the snapshot feature was included. I would be on board with continuing to support it with a whitelist notification.

dwsupplee reviewed

View reviewed changes

Contributor

dwsupplee left a comment

The only GAPIC I spent much time looking at was DLP, as I assume most of the changes are the same across the board. If there is anything special I should go back and take a look at please let me know 😄.

src/Core/GrpcRequestWrapper.php Outdated

+                          $retrySettings = [
+                              'retriesEnabled' => false
+                          ];
+                          if ($timeout && !array_key_exists('timeoutMs', $grpcOptions)) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

@@ @@ -124,6 +124,9 @@ class DlpServiceGapicClient @@
                   const CODEGEN_VERSION = '0.0.5';
                   private static $resultNameTemplate;
+                  private static $pathTemplateList = null;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

@@ @@ -170,6 +143,16 @@ private static function getResultNameTemplate() @@
                       return self::$resultNameTemplate;
                   }
+                  private static function getPathTemplateList()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

+                      if (!self::$gapicVersionLoaded) {
+                          if (file_exists(__DIR__.'/../VERSION')) {
+                              self::$gapicVersion = trim(file_get_contents(__DIR__.'/../VERSION'));
+                          } elseif (class_exists('\Google\Cloud\Version')) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

+                          if (file_exists(__DIR__.'/../VERSION')) {
+                              self::$gapicVersion = trim(file_get_contents(__DIR__.'/../VERSION'));
+                          } elseif (class_exists('\Google\Cloud\Version')) {
+                              self::$gapicVersion = \Google\Cloud\Version::VERSION;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

		@@ -278,6 +305,7 @@ public function __construct($options = [])
		'timeoutMillis' => self::DEFAULT_TIMEOUT_MILLIS,

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php

                    *           A CredentialsLoader object created using the Google\Auth library.
                    *     @type array $scopes A string array of scopes to use when acquiring credentials.
                    *                          Defaults to the scopes for the DLP API.
+                   *     @type string $clientConfigPath

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

+                   * @return array An associative array from name component IDs to component values.
+                   * @experimental
+                   */
+                  public static function parseName($formattedName)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/Spanner/Database.php Outdated

@@ @@ -309,7 +309,9 @@ public function create(array $options = []) @@
                           'statements' => [],
                       ];
-                      $statement = sprintf('CREATE DATABASE `%s`', DatabaseAdminClient::parseDatabaseFromDatabaseName($this->name));
+                      $databaseNameComponents = DatabaseAdminClient::parseName($this->name());

This comment was marked as spam.

Sign in to view

src/Speech/V1/Gapic/SpeechGapicClient.php Outdated

                    */
                   public function streamingRecognize($optionalArgs = [])
                   {
+                      if (array_key_exists('timeoutMillis', $optionalArgs)) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

michaelbausor added 4 commits

September 19, 2017 09:00


          Address PR comments

a89299c


          Update GAPICs with changes to toolkit to address PR comments

72e62f1


          Update gapics

d3b5b60


          Switch use of parse method

8edf5c4

jdpedrie reviewed

View reviewed changes

src/Dlp/V2beta1/Gapic/DlpServiceGapicClient.php Outdated

-                  {
-                      if (self::$resultNameTemplate == null) {
-                          self::$resultNameTemplate = new PathTemplate('inspect/results/{result}');
+                      if (isset($templateMap[$template])) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

michaelbausor added 2 commits

September 19, 2017 11:28


          Refresh with toolkit changes


          Restore UpdateSubsciption

1389b88

dwsupplee reviewed

View reviewed changes

Contributor

dwsupplee left a comment

One small note, otherwise LGTM!

src/PubSub/Subscription.php Outdated

@@ @@ -223,6 +223,52 @@ public function create(array $options = []) @@
                   }
                   /**
+                   * Update the subscription.
+                   *
+                   * Note that subscription name and topic are immutable properties and may

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

michaelbausor added 2 commits

September 19, 2017 13:19


          Trigger CI

8408e9f


          Empty commit to trigger CI

ade1a71

dwsupplee merged commit c94a198 into googleapis:master

dwsupplee mentioned this pull request

Restore subscription update tests #695

Merged

jdpedrie mentioned this pull request

Prepare v0.40.0 #705

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels