googleapis / gax-ruby

Google API Extensions for Ruby
https://rubygems.org/gems/google-gax
BSD 3-Clause "New" or "Revised" License
20 stars 22 forks source link

Lookup Operation types to unpack #134

Closed blowmage closed 6 years ago

blowmage commented 6 years ago

This PR changes Google::Gax::Operation to lookup the result and metadata types used to unpack the values from the Google::LongRunning::Operation object. This only happens when the result_type or metadata_type values are passed in as nil (which currently results in an error).

This change will enable the GAPIC client objects to retrieving operations through a get_operation and a list_operation method without having to know the specific result and metadata types to set. There are several APIs that have operation collections which contain resources with varied result and metadata types. Each Google::LongRunning::Operation object would have to be inspected and the result and metadata types discovered before passing to Google::Gax::Operation.new. This PR moves the result and metadata types discovery to Google::Gax::Operation.

codecov-io commented 6 years ago

Codecov Report

Merging #134 into master will increase coverage by 0.05%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   99.35%   99.41%   +0.05%     
==========================================
  Files          19       19              
  Lines        2029     2058      +29     
==========================================
+ Hits         2016     2046      +30     
+ Misses         13       12       -1
Impacted Files Coverage Δ
lib/google/gax/bundling.rb 99.37% <ø> (ø) :arrow_up:
spec/google/gax/operation_spec.rb 100% <100%> (ø) :arrow_up:
lib/google/gax/operation.rb 100% <100%> (ø) :arrow_up:
spec/google/gax/bundling_spec.rb 100% <0%> (+0.3%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e29a2ad...682f872. Read the comment docs.

blowmage commented 6 years ago

I would love if Operation always looked up the types, and ignored the types passed in. But I felt that might be too big of a change so this PR is a more conservative approach that will always use the provided types when given. I can rework this PR to ignore the provided types if want, or I can open a competing PR that does that.

blowmage commented 6 years ago

With this change, the GAPIC generators could be changed to specify nil to activate the lookup everytime #metadata or #response is called.

def long_running_recognize \
    config,
    audio,
    options: nil
  req = {
    config: config,
    audio: audio
  }.delete_if { |_, v| v.nil? }
  req = Google::Gax::to_proto(req, Google::Cloud::Speech::V1::LongRunningRecognizeRequest)
  operation = Google::Gax::Operation.new(
    @long_running_recognize.call(req, options),
    @operations_client,
    nil, # was Google::Cloud::Speech::V1::LongRunningRecognizeResponse
    nil, # was Google::Cloud::Speech::V1::LongRunningRecognizeMetadata
    call_options: options
  )
  operation.on_done { |operation| yield(operation) } if block_given?
  operation
end