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

Add GAX call_options to methods making API calls #849

Closed
wants to merge 1 commit into from

Conversation

blowmage
Copy link
Contributor

@blowmage blowmage commented Aug 24, 2016

This is an attempt to allow users more control over the behavior of the client making API calls. GAX implements retries and incremental backoff, but the GAX configuration is a bit different than what is used by other services. This is an option to allow users to override the GAX configuration on the API call while continuing to use Google Cloud. #848 allows users to configure the client settings, but this PR is for configuring the API call settings. Either approach is independent of the other.

This change is additive, but it does leak the GAX implementation to the Google Cloud Language public API. This means that if GAX ever changes its implementation Google Cloud Language may need to also make a breaking change to support the new GAX API. So far we have resisted leaking these types of details, but there is no better way to allow the GAX client to be fully configurable other than leaking these details.

What do you think of this type of change? Is this a direction you want to see the other GRPC/GAX services move towards?

The call_config hash can contain any part of the GAX CallOptions object structure. Here is what this may look like:

my_options = {"retry_options"=>
               {"backoff_settings"=>
                 {"initial_retry_delay_millis"=>100,
                  "retry_delay_multiplier"=>1.3,
                  "max_retry_delay_millis"=>60000,
                  "initial_rpc_timeout_millis"=>60000,
                  "rpc_timeout_multiplier"=>1.0,
                  "max_rpc_timeout_millis"=>60000,
                  "total_timeout_millis"=>600000},
                "retry_codes"=>["DEADLINE_EXCEEDED", "UNAVAILABLE"]}

require "google/cloud"

gcloud = Google::Cloud.new
results = gcloud.language.annotate "Hello world!", call_options: my_options

Allow for GRPC/GAX behavior to be configured on methods that make API calls.
This leaks the GAX implementation to the google-cloud public API.
If Gax changes its implementation this may result in a change to
the google-cloud public API.
@blowmage blowmage added the api: language Issues related to the Cloud Natural Language API API. label Aug 24, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 24, 2016
@blowmage
Copy link
Contributor Author

@timanovsky This also isn't Datastore, but this is also the approach we are considering for configuring GRPC/GAX call requests. Can you look at this and let us know what you think of this? Would you prefer leaking the GAX client configuration over the GAX call configuration? Do you need/want both?

@blowmage
Copy link
Contributor Author

@jmuk Can you also weigh in on this? How comfortable are you with gcloud-ruby leaking the GAX call configuration? How likely are we to see breaking changes to the call configuration? Would love to get your thoughts.

@blowmage
Copy link
Contributor Author

@omaray @jgeewax Thoughts on this?

@blowmage blowmage mentioned this pull request Aug 24, 2016
6 tasks
@blowmage
Copy link
Contributor Author

@bmclean Or this?

@coveralls
Copy link

coveralls commented Aug 25, 2016

Coverage Status

Coverage decreased (-0.01%) to 95.645% when pulling 31c6a15 on blowmage:language-call_options into 956fbba on GoogleCloudPlatform:master.

@bmclean
Copy link
Contributor

bmclean commented Aug 25, 2016

I commented on #848.

@timanovsky
Copy link

timanovsky commented Sep 12, 2016

Sorry, for the late comment on this. As an API user I'm ok having call_options parameter in each API method. But I guess it can become a mess in library code. Did you consider alterantive like:

gcloud.with_call_options(my_options) do
   results = gcloud.language.annotate "Hello world!"
end

One catch is to implement it in a thread safe way.

@quartzmo
Copy link
Member

We can re-open this if the need arises, but closing for now.

@quartzmo quartzmo closed this Nov 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: language Issues related to the Cloud Natural Language API API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants