From 3e08e467cd9bf7d6eccfc6c7dcc3051033a971f2 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 30 Sep 2016 11:02:59 -0600 Subject: [PATCH] Add GAX client_config to Datastore service factory Replace retries value with the GAX client configuration. This leaks the GAX implementation to the public API. If Gax changes its implementation this may result in a change to the google-cloud-datastore public API. --- .../acceptance/datastore_helper.rb | 2 +- .../lib/google-cloud-datastore.rb | 22 ++++++------ .../lib/google/cloud/datastore.rb | 30 +++++++++------- .../lib/google/cloud/datastore/service.rb | 34 ++++++++++--------- .../lib/google/cloud/datastore/v1.rb | 2 ++ .../test/google/cloud/datastore_test.rb | 22 ++++++------ 6 files changed, 61 insertions(+), 51 deletions(-) diff --git a/google-cloud-datastore/acceptance/datastore_helper.rb b/google-cloud-datastore/acceptance/datastore_helper.rb index 4b507899fe5d..72e4c244088a 100644 --- a/google-cloud-datastore/acceptance/datastore_helper.rb +++ b/google-cloud-datastore/acceptance/datastore_helper.rb @@ -19,7 +19,7 @@ require "google/cloud/datastore" # Create shared dataset object so we don't create new for each test -$dataset = Google::Cloud.new.datastore retries: 10 +$dataset = Google::Cloud.new.datastore module Acceptance ## diff --git a/google-cloud-datastore/lib/google-cloud-datastore.rb b/google-cloud-datastore/lib/google-cloud-datastore.rb index a2a56d0f2435..52d5fc6b8375 100644 --- a/google-cloud-datastore/lib/google-cloud-datastore.rb +++ b/google-cloud-datastore/lib/google-cloud-datastore.rb @@ -38,8 +38,9 @@ module Cloud # The default scope is: # # * `https://www.googleapis.com/auth/datastore` - # @param [Integer] retries This option is not currently supported. # @param [Integer] timeout Default timeout to use in requests. Optional. + # @param [Hash] client_config A hash of values to override the default + # behavior of the API client. See Google::Gax::CallSettings. Optional. # # @return [Google::Cloud::Datastore::Dataset] # @@ -65,10 +66,10 @@ module Cloud # platform_scope = "https://www.googleapis.com/auth/cloud-platform" # datastore = gcloud.datastore scope: platform_scope # - def datastore scope: nil, retries: nil, timeout: nil + def datastore scope: nil, timeout: nil, client_config: nil Google::Cloud.datastore @project, @keyfile, - scope: scope, retries: retries, - timeout: (timeout || @timeout) + scope: scope, timeout: (timeout || @timeout), + client_config: client_config end ## @@ -90,8 +91,9 @@ def datastore scope: nil, retries: nil, timeout: nil # The default scope is: # # * `https://www.googleapis.com/auth/datastore` - # @param [Integer] retries This option is not currently supported. # @param [Integer] timeout Default timeout to use in requests. Optional. + # @param [Hash] client_config A hash of values to override the default + # behavior of the API client. See Google::Gax::CallSettings. Optional. # # @return [Google::Cloud::Datastore::Dataset] # @@ -106,16 +108,16 @@ def datastore scope: nil, retries: nil, timeout: nil # t["done"] = false # t["priority"] = 4 # t["description"] = "Learn Cloud Datastore" - # end + # end `` # # datastore.save task # - def self.datastore project = nil, keyfile = nil, scope: nil, retries: nil, - timeout: nil + def self.datastore project = nil, keyfile = nil, scope: nil, timeout: nil, + client_config: nil require "google/cloud/datastore" Google::Cloud::Datastore.new project: project, keyfile: keyfile, - scope: scope, retries: retries, - timeout: timeout + scope: scope, timeout: timeout, + client_config: client_config end end end diff --git a/google-cloud-datastore/lib/google/cloud/datastore.rb b/google-cloud-datastore/lib/google/cloud/datastore.rb index 026b65f011a9..182cc7bf879a 100644 --- a/google-cloud-datastore/lib/google/cloud/datastore.rb +++ b/google-cloud-datastore/lib/google/cloud/datastore.rb @@ -518,9 +518,9 @@ module Datastore # The default scope is: # # * `https://www.googleapis.com/auth/datastore` - # @param [Integer] retries Number of times to retry requests on server - # error. The default value is `3`. Optional. # @param [Integer] timeout Default timeout to use in requests. Optional. + # @param [Hash] client_config A hash of values to override the default + # behavior of the API client. See Google::Gax::CallSettings. Optional. # # @return [Google::Cloud::Datastore::Dataset] # @@ -541,8 +541,8 @@ module Datastore # # datastore.save task # - def self.new project: nil, keyfile: nil, scope: nil, retries: nil, - timeout: nil + def self.new project: nil, keyfile: nil, scope: nil, timeout: nil, + client_config: nil project ||= Google::Cloud::Datastore::Dataset.default_project project = project.to_s # Always cast to a string fail ArgumentError, "project is missing" if project.empty? @@ -551,20 +551,24 @@ def self.new project: nil, keyfile: nil, scope: nil, retries: nil, return Google::Cloud::Datastore::Dataset.new( Google::Cloud::Datastore::Service.new( project, :this_channel_is_insecure, - host: ENV["DATASTORE_EMULATOR_HOST"], retries: retries)) + host: ENV["DATASTORE_EMULATOR_HOST"], + client_config: client_config)) end + credentials = credentials_with_scope keyfile, scope + Google::Cloud::Datastore::Dataset.new( + Google::Cloud::Datastore::Service.new( + project, credentials, timeout: timeout, + client_config: client_config)) + end + ## + # @private + def self.credentials_with_scope keyfile, scope if keyfile.nil? - credentials = Google::Cloud::Datastore::Credentials.default( - scope: scope) + Google::Cloud::Datastore::Credentials.default(scope: scope) else - credentials = Google::Cloud::Datastore::Credentials.new( - keyfile, scope: scope) + Google::Cloud::Datastore::Credentials.new(keyfile, scope: scope) end - - Google::Cloud::Datastore::Dataset.new( - Google::Cloud::Datastore::Service.new( - project, credentials, retries: retries, timeout: timeout)) end end end diff --git a/google-cloud-datastore/lib/google/cloud/datastore/service.rb b/google-cloud-datastore/lib/google/cloud/datastore/service.rb index 809a7ef3dee9..696360fba1bb 100644 --- a/google-cloud-datastore/lib/google/cloud/datastore/service.rb +++ b/google-cloud-datastore/lib/google/cloud/datastore/service.rb @@ -16,8 +16,8 @@ require "google/cloud/errors" require "google/cloud/datastore/credentials" require "google/cloud/datastore/version" -require "google/datastore/v1/datastore_services_pb" -require "google/cloud/datastore/v1/datastore_api" +require "google/cloud/datastore/v1" +require "google/gax/errors" module Google module Cloud @@ -26,38 +26,40 @@ module Datastore # @private Represents the GAX Datastore service, including all the API # methods. class Service - attr_accessor :project, :credentials, :host, :retries, :timeout + attr_accessor :project, :credentials, :host, :timeout, :client_config ## # Creates a new Service instance. - def initialize project, credentials, host: nil, retries: nil, - timeout: nil + def initialize project, credentials, host: nil, timeout: nil, + client_config: nil @project = project @credentials = credentials @host = host || V1::DatastoreApi::SERVICE_ADDRESS - @retries = retries @timeout = timeout + @client_config = client_config || {} end def channel + require "grpc" GRPC::Core::Channel.new host, nil, chan_creds end def chan_creds return credentials if insecure? + require "grpc" GRPC::Core::ChannelCredentials.new.compose \ GRPC::Core::CallCredentials.new credentials.client.updater_proc end def service return mocked_service if mocked_service - @service ||= \ - V1::DatastoreApi.new( - service_path: host, - channel: channel, - timeout: timeout, - app_name: "google-cloud-datastore", - app_version: Google::Cloud::Datastore::VERSION) + @service ||= V1::DatastoreApi.new( + service_path: host, + channel: channel, + timeout: timeout, + client_config: client_config, + app_name: "google-cloud-datastore", + app_version: Google::Cloud::Datastore::VERSION) end attr_accessor :mocked_service @@ -143,10 +145,10 @@ def generate_read_options consistency, transaction end def execute - require "grpc" # Ensure GRPC is loaded before rescuing exception yield - rescue GRPC::BadStatus => e - raise Google::Cloud::Error.from_error(e) + rescue Google::Gax::GaxError => e + # GaxError wraps BadStatus, but exposes it as #cause + raise Google::Cloud::Error.from_error(e.cause) end end end diff --git a/google-cloud-datastore/lib/google/cloud/datastore/v1.rb b/google-cloud-datastore/lib/google/cloud/datastore/v1.rb index c9cec948018a..2d46f03fcea3 100644 --- a/google-cloud-datastore/lib/google/cloud/datastore/v1.rb +++ b/google-cloud-datastore/lib/google/cloud/datastore/v1.rb @@ -14,3 +14,5 @@ require "google/cloud/datastore/v1/datastore_api" +# Require the protobufs so we can create objects before GRPC is loaded. +require "google/datastore/v1/datastore_pb" diff --git a/google-cloud-datastore/test/google/cloud/datastore_test.rb b/google-cloud-datastore/test/google/cloud/datastore_test.rb index 8eab798a5797..1762e6f28f86 100644 --- a/google-cloud-datastore/test/google/cloud/datastore_test.rb +++ b/google-cloud-datastore/test/google/cloud/datastore_test.rb @@ -19,12 +19,12 @@ describe "#datastore" do it "calls out to Google::Cloud.datastore" do gcloud = Google::Cloud.new - stubbed_datastore = ->(project, keyfile, scope: nil, retries: nil, timeout: nil) { + stubbed_datastore = ->(project, keyfile, scope: nil, timeout: nil, client_config: nil) { project.must_equal nil keyfile.must_equal nil scope.must_be :nil? - retries.must_be :nil? timeout.must_be :nil? + client_config.must_be :nil? "datastore-dataset-object-empty" } Google::Cloud.stub :datastore, stubbed_datastore do @@ -35,12 +35,12 @@ it "passes project and keyfile to Google::Cloud.datastore" do gcloud = Google::Cloud.new "project-id", "keyfile-path" - stubbed_datastore = ->(project, keyfile, scope: nil, retries: nil, timeout: nil) { + stubbed_datastore = ->(project, keyfile, scope: nil, timeout: nil, client_config: nil) { project.must_equal "project-id" keyfile.must_equal "keyfile-path" scope.must_be :nil? - retries.must_be :nil? timeout.must_be :nil? + client_config.must_be :nil? "datastore-dataset-object" } Google::Cloud.stub :datastore, stubbed_datastore do @@ -51,16 +51,16 @@ it "passes project and keyfile and options to Google::Cloud.datastore" do gcloud = Google::Cloud.new "project-id", "keyfile-path" - stubbed_datastore = ->(project, keyfile, scope: nil, retries: nil, timeout: nil) { + stubbed_datastore = ->(project, keyfile, scope: nil, timeout: nil, client_config: nil) { project.must_equal "project-id" keyfile.must_equal "keyfile-path" scope.must_equal "http://example.com/scope" - retries.must_equal 5 timeout.must_equal 60 + client_config.must_equal({ "gax" => "options" }) "datastore-dataset-object-scoped" } Google::Cloud.stub :datastore, stubbed_datastore do - dataset = gcloud.datastore scope: "http://example.com/scope", retries: 5, timeout: 60 + dataset = gcloud.datastore scope: "http://example.com/scope", timeout: 60, client_config: { "gax" => "options" } dataset.must_equal "datastore-dataset-object-scoped" end end @@ -91,11 +91,11 @@ scope.must_equal nil "datastore-credentials" } - stubbed_service = ->(project, credentials, retries: nil, timeout: nil) { + stubbed_service = ->(project, credentials, timeout: nil, client_config: nil) { project.must_equal "project-id" credentials.must_equal "datastore-credentials" - retries.must_equal nil timeout.must_equal nil + client_config.must_equal nil OpenStruct.new project: project } @@ -142,11 +142,11 @@ scope.must_equal nil "datastore-credentials" } - stubbed_service = ->(project, credentials, retries: nil, timeout: nil) { + stubbed_service = ->(project, credentials, timeout: nil, client_config: nil) { project.must_equal "project-id" credentials.must_equal "datastore-credentials" - retries.must_equal nil timeout.must_equal nil + client_config.must_equal nil OpenStruct.new project: project }