feat(bigquery): Add support for ML model export #7451

quartzmo · 2020-08-12T21:47:59Z

closes: #7061

Add model support to Project#extract and Project#extract_job
Add ExtractJob#model?
Add ExtractJob#table?
Add ExtractJob#ml_tf_saved_model?
Add ExtractJob#ml_xgboost_booster?
Add Model#extract
Add Model#extract_job

* Add model support to Project#extract and #extract_job * Add ExtractJob#model? * Add ExtractJob#ml_tf_saved_model? * Add ExtractJob#ml_xgboost_booster? * Add Model#extract and #extract_job closes: googleapis#7061

dazuma · 2020-08-17T16:52:34Z

google-cloud-bigquery/acceptance/bigquery/bigquery_test.rb

+      Tempfile.open "temp_extract_model" do |tmp|
+        extract_url = "gs://#{bucket.name}/#{model_id}"
+
+        # sut


what does this mean?

just shorthand for the method being tested

http://xunitpatterns.com/SUT.html

dazuma · 2020-08-17T17:09:33Z

google-cloud-bigquery/lib/google/cloud/bigquery/extract_job.rb

        #
        # @return [Boolean] `true` when `CSV`, `false` otherwise.
        #
        def csv?
          val = @gapi.configuration.extract.destination_format
-          return true if val.nil?
+          return true if table? && val.nil?


Should we make explicit that false is returned when extracting models? (i.e. documenting it, and maybe the code could say return false unless table? instead which would be more clear than this which assumes that val is nil when extracting models).

Similar comment for the similar methods surrounding this.

dazuma · 2020-08-17T17:28:43Z

google-cloud-bigquery/lib/google/cloud/bigquery/extract_job.rb

@@ -253,7 +319,7 @@ def location= value
          end

          ##
-          # Sets the compression type.
+          # Sets the compression type. Not applicable when extracting models.


Would it cause any problem if set when extracting models? (Should we just leave it be, or throw an exception?)

Our usual policy is to pass it through to the backend service.

Yeah, this is a good example of letting the backend sort it out, as they may add support for this down the line.

dazuma · 2020-08-17T17:33:21Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        ##
+        # Exports the model to a Google Cloud Storage file using
+        # an asynchronous method. In this method, an {ExtractJob} is immediately
+        # returned. The caller may poll the service by repeatedly calling


Maybe: Exports the model to a Google CloudStorage file asynchronously, immediately returning an {ExtractJob} that can be used to track the progress of the export job. The caller may poll the service...

Much improved, thank you.

dazuma · 2020-08-17T17:35:33Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        # The geographic location for the job ("US", "EU", etc.) can be set via
+        # {ExtractJob::Updater#location=} in a block passed to this method. If
+        # the model is a full resource representation (see {#resource_full?}),
+        # the location of the job will be automatically set to the location of


nit: ...will automatically be set...

dazuma · 2020-08-17T17:37:01Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        #   be used.
+        # @param [Hash] labels A hash of user-provided labels associated with
+        #   the job. You can use these to organize and group your jobs. Label
+        #   keys and values can be no longer than 63 characters, can only


nit: ...63 characters, and can only...

dazuma · 2020-08-17T17:37:36Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        # @param [Hash] labels A hash of user-provided labels associated with
+        #   the job. You can use these to organize and group your jobs. Label
+        #   keys and values can be no longer than 63 characters, can only
+        #   contain lowercase letters, numeric characters, underscores and


In other param descriptions you include letters (a-z), etc. examples. Include them here too?

I decided to copy in the entirety of the latest version of the label requirements from https://cloud.google.com/bigquery/docs/labels-intro#requirements

dazuma · 2020-08-17T18:11:20Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        #   the job. You can use these to organize and group your jobs. Label
+        #   keys and values can be no longer than 63 characters, can only
+        #   contain lowercase letters, numeric characters, underscores and
+        #   dashes. International characters are allowed. Label values are


What does international characters mean? (e.g. does this mean only "lower case" international characters? Does that exclude alphabets that do not have the concept of upper and lower case?)

I'm not sure, I'll see if the official documentation offers anything.

It turns out the link below is outdated and now goes to the wrong page. Will fix.

I decided to copy in the entirety of the latest version of the label requirements from https://cloud.google.com/bigquery/docs/labels-intro#requirements, and omit the link.

dazuma · 2020-08-17T18:12:54Z

google-cloud-bigquery/lib/google/cloud/bigquery/model.rb

+        #   labels](https://cloud.google.com/bigquery/docs/creating-managing-labels#requirements).
+        # @param [Boolean] dryrun  If set, don't actually run this job. Behavior
+        #   is undefined however for non-query jobs and may result in an error.
+        #   Deprecated.


This method is a new one being added. Given that, can we just omit this argument rather than creating it for the first time as already deprecated?

Good catch!

dazuma

LGTM

quartzmo · 2020-08-24T18:05:05Z

@shollyman PTAL, I believe this PR is ready to merge.

shollyman

Thanks for putting this all together. The testing scenarios have great coverage, checking for the the TF saved model etc.

I feel somehow guilty for how the level of repetition this request induced. So many rewrites of the label validation rule comments. :)

shollyman · 2020-09-01T03:18:33Z

google-cloud-bigquery/lib/google/cloud/bigquery/extract_job.rb

@@ -253,7 +319,7 @@ def location= value
          end

          ##
-          # Sets the compression type.
+          # Sets the compression type. Not applicable when extracting models.


Yeah, this is a good example of letting the backend sort it out, as they may add support for this down the line.

Add Model Export support

4fc030b

closes: googleapis#7061

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 12, 2020

feat(bigquery): Add support for ML model export

eabcbfe

* Add model support to Project#extract and #extract_job * Add ExtractJob#model? * Add ExtractJob#ml_tf_saved_model? * Add ExtractJob#ml_xgboost_booster? * Add Model#extract and #extract_job closes: googleapis#7061

quartzmo force-pushed the bigquery-model-extract branch from 3e1312d to eabcbfe Compare August 12, 2020 21:49

quartzmo self-assigned this Aug 12, 2020

Update ExtractJob to support Model

de92305

quartzmo force-pushed the bigquery-model-extract branch from 93961be to de92305 Compare August 12, 2020 22:35

quartzmo added the api: bigquery Issues related to the BigQuery API. label Aug 12, 2020

quartzmo added 2 commits August 13, 2020 11:53

Update external docs links

4d08141

Update extract docs for models

3e4e5f3

quartzmo marked this pull request as ready for review August 13, 2020 18:13

quartzmo requested a review from a team as a code owner August 13, 2020 18:13

dazuma requested changes Aug 17, 2020

View reviewed changes

quartzmo added 3 commits August 17, 2020 15:42

Update conditional returns and docs

59f687a

Remove dry_run, update docs, and add conditional returns

1dc5ceb

Update labels requirements in docs

3cf1dc3

dazuma approved these changes Aug 18, 2020

View reviewed changes

shollyman approved these changes Sep 1, 2020

View reviewed changes

quartzmo merged commit 7626b17 into googleapis:master Sep 1, 2020

quartzmo deleted the bigquery-model-extract branch September 1, 2020 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bigquery): Add support for ML model export #7451

feat(bigquery): Add support for ML model export #7451

quartzmo commented Aug 12, 2020 •

edited

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

shollyman Sep 1, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

quartzmo Aug 17, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

dazuma Aug 17, 2020

quartzmo Aug 18, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

quartzmo Aug 18, 2020

quartzmo Aug 18, 2020

dazuma Aug 17, 2020

quartzmo Aug 17, 2020

quartzmo Aug 17, 2020

dazuma left a comment

quartzmo commented Aug 24, 2020

shollyman left a comment

shollyman Sep 1, 2020

feat(bigquery): Add support for ML model export #7451

feat(bigquery): Add support for ML model export #7451

Conversation

quartzmo commented Aug 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dazuma left a comment

Choose a reason for hiding this comment

quartzmo commented Aug 24, 2020

shollyman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quartzmo commented Aug 12, 2020 •

edited