ci(speech): update quickstart to V2 demonstrating regional endpoints #13760

scotthart · 2024-03-11T16:18:27Z

While we need to wait until after the next release to commit these changes, for our own CI purposes, this quickstart shows how to use the API available at HEAD to direct the speech client to use regional endpoints and reocgnizers.

part of the work for #13729

blocked on microsoft/vcpkg#38204

This change is

google/cloud/speech/quickstart/quickstart.cc

codecov · 2024-03-11T17:03:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.78%. Comparing base (cb500d5) to head (aba30e7).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13760      +/-   ##
==========================================
+ Coverage   93.23%   93.78%   +0.55%     
==========================================
  Files        2197     2284      +87     
  Lines      188700   199141   +10441     
==========================================
+ Hits       175929   186762   +10833     
+ Misses      12771    12379     -392

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

scotthart

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @devbww)

google/cloud/speech/quickstart/quickstart.cc line 37 at r1 (raw file):

Previously, devbww (Bradley White) wrote…

What if argc is 0, 1, or 2?

Added additional argc usage condition.

google/cloud/speech/quickstart/quickstart.cc line 54 at r1 (raw file):

Previously, devbww (Bradley White) wrote…

Factor out the set_recognizer.

[Then the MakeSpeechConnection() can be turned into a conditional expression and become the connection initializer.]

Moved set recognizer out of the conditional.

devbww · 2024-03-11T17:49:47Z

google/cloud/speech/README.md

 int main(int argc, char* argv[]) try {
  auto constexpr kDefaultUri = "gs://cloud-samples-data/speech/hello.wav";
-  if (argc > 2) {
-    std::cerr << "Usage: " << argv[0] << " [gcs-uri]\n"
+  if (argc < 3 || argc > 4) {


Nit: (argc != 3 && argc != 4) would be the clearer exposition, methinks.

devbww · 2024-03-11T17:51:07Z

google/cloud/speech/README.md

+  if (location == "global") {
+    connection = speech::MakeSpeechConnection();
+  } else {
+    connection = speech::MakeSpeechConnection(location);
+  }


Opt: Perhaps MakeXxxConnection("global") should mean the same thing as MakeXxxConnection()?

scotthart

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @devbww)

google/cloud/speech/README.md line 39 at r2 (raw file):

Previously, devbww (Bradley White) wrote…

Nit: (argc != 3 && argc != 4) would be the clearer exposition, methinks.

Done.

google/cloud/speech/README.md line 62 at r2 (raw file):

Previously, devbww (Bradley White) wrote…

Opt: Perhaps MakeXxxConnection("global") should mean the same thing as MakeXxxConnection()?

I'd rather have the overload than introduce a sentinel value.

devbww

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @scotthart)

google/cloud/speech/README.md line 62 at r2 (raw file):

Previously, scotthart (Scott Hart) wrote…

I'd rather have the overload than introduce a sentinel value.

Note that we already have a sentinel of an empty location.

google-cloud-cpp/google/cloud/speech/v2/internal/speech_option_defaults.cc

Line 41 in 0f71367

absl::StrCat(location, location.empty() ? "" : "-",

And we'd still have the overload for the "compat" part.

scotthart

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @devbww)

google/cloud/speech/README.md line 62 at r2 (raw file):

Previously, devbww (Bradley White) wrote…

Note that we already have a sentinel of an empty location.

google-cloud-cpp/google/cloud/speech/v2/internal/speech_option_defaults.cc

Line 41 in 0f71367

absl::StrCat(location, location.empty() ? "" : "-",

And we'd still have the overload for the "compat" part.

Use the non-deprecated MakeConnection function with the existing sentinel value.

dbolduc · 2024-04-26T16:23:49Z

ci/etc/integration-tests-config.sh

+# Some tests and quickstarts need to specify a location as "global".
+export GOOGLE_CLOUD_CPP_TEST_GLOBAL="global"


This can only ever have one value ("global") so I don't think having a variable makes sense. Let's just hardcode "global" in the quickstart runner.

dbolduc · 2024-04-26T16:27:22Z

google/cloud/speech/quickstart/quickstart.cc

+  connection = speech::MakeSpeechConnection(location);
+  auto client = speech::SpeechClient(connection);


I think 99% of the quickstarts squash this into one line.

auto client = speech::SpeechClient(speech::MakeSpeechConnection(location));

dbolduc · 2024-04-29T15:17:14Z

google/cloud/speech/CMakeLists.txt

@@ -26,12 +26,20 @@ if (BUILD_TESTING AND GOOGLE_CLOUD_CPP_ENABLE_CXX_EXCEPTIONS)
    target_link_libraries(speech_quickstart PRIVATE google-cloud-cpp::speech)
    google_cloud_cpp_add_common_options(speech_quickstart)
    add_test(


this is cool. good idea to have multiple tests for the quickstart

scotthart

Reviewable status: 0 of 4 files reviewed, 4 unresolved discussions (waiting on @dbolduc and @devbww)

google/cloud/speech/CMakeLists.txt line 28 at r4 (raw file):

Previously, dbolduc (Darren Bolduc) wrote…

this is cool. good idea to have multiple tests for the quickstart

Done.

google/cloud/speech/quickstart/quickstart.cc line 55 at r4 (raw file):

Previously, dbolduc (Darren Bolduc) wrote…

I think 99% of the quickstarts squash this into one line.
auto client = speech::SpeechClient(speech::MakeSpeechConnection(location));

Done.

ci/etc/integration-tests-config.sh line 38 at r4 (raw file):

Previously, dbolduc (Darren Bolduc) wrote…

This can only ever have one value ("global") so I don't think having a variable makes sense. Let's just hardcode "global" in the quickstart runner.

Done.

…oogleapis#13760)

scotthart temporarily deployed to internal March 11, 2024 16:18 — with GitHub Actions Inactive

product-auto-label bot added the api: speech Issues related to the Speech-to-Text API. label Mar 11, 2024

devbww reviewed Mar 11, 2024

View reviewed changes

google/cloud/speech/quickstart/quickstart.cc Outdated Show resolved Hide resolved

google/cloud/speech/quickstart/quickstart.cc Outdated Show resolved Hide resolved

scotthart temporarily deployed to internal March 11, 2024 17:34 — with GitHub Actions Inactive

scotthart commented Mar 11, 2024

View reviewed changes

devbww reviewed Mar 11, 2024

View reviewed changes

scotthart temporarily deployed to internal March 11, 2024 18:09 — with GitHub Actions Inactive

scotthart commented Mar 11, 2024

View reviewed changes

devbww reviewed Mar 11, 2024

View reviewed changes

alevenberg added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Apr 15, 2024

scotthart force-pushed the speech_v2_update_quickstart branch from 4e283d8 to 9ac2f7b Compare April 19, 2024 17:30

scotthart temporarily deployed to internal April 19, 2024 17:30 — with GitHub Actions Inactive

scotthart force-pushed the speech_v2_update_quickstart branch from 9ac2f7b to 69410ae Compare April 24, 2024 15:42

scotthart temporarily deployed to internal April 24, 2024 15:42 — with GitHub Actions Inactive

scotthart force-pushed the speech_v2_update_quickstart branch from 69410ae to f665ff2 Compare April 25, 2024 17:33

scotthart temporarily deployed to internal April 25, 2024 17:33 — with GitHub Actions Inactive

scotthart temporarily deployed to internal April 26, 2024 14:34 — with GitHub Actions Inactive

scotthart marked this pull request as ready for review April 26, 2024 14:34

scotthart requested a review from a team as a code owner April 26, 2024 14:34

scotthart commented Apr 26, 2024

View reviewed changes

devbww approved these changes Apr 26, 2024

View reviewed changes

dbolduc reviewed Apr 29, 2024

View reviewed changes

scotthart temporarily deployed to internal April 29, 2024 16:17 — with GitHub Actions Inactive

scotthart commented Apr 29, 2024

View reviewed changes

dbolduc approved these changes Apr 29, 2024

View reviewed changes

scotthart added 3 commits April 29, 2024 12:22

ci(speech): update quickstart to V2 demonstrating regional endpoints

d19080b

address review comments

920bfa3

refactor compound conditional

123ceed

scotthart added 2 commits April 29, 2024 12:22

use non-deprecated MakeConnection overload with empty location

724098a

address review comments

0dcce9b

scotthart force-pushed the speech_v2_update_quickstart branch from e50c2b9 to 0dcce9b Compare April 29, 2024 16:22

scotthart temporarily deployed to internal April 29, 2024 16:23 — with GitHub Actions Inactive

cmake add_test requires arguments as variables

aba30e7

scotthart temporarily deployed to internal April 29, 2024 17:38 — with GitHub Actions Inactive

scotthart enabled auto-merge (squash) April 29, 2024 17:59

scotthart merged commit 226c607 into googleapis:main Apr 29, 2024
63 of 64 checks passed

devbww pushed a commit to devbww/google-cloud-cpp that referenced this pull request May 1, 2024

ci(speech): update quickstart to V2 demonstrating regional endpoints (g…

b11b1d7

…oogleapis#13760)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(speech): update quickstart to V2 demonstrating regional endpoints #13760

ci(speech): update quickstart to V2 demonstrating regional endpoints #13760

scotthart commented Mar 11, 2024 •

edited

Loading

codecov bot commented Mar 11, 2024 •

edited

Loading

scotthart left a comment

devbww Mar 11, 2024

devbww Mar 11, 2024

scotthart left a comment

devbww left a comment

scotthart left a comment

dbolduc Apr 26, 2024

dbolduc Apr 26, 2024

dbolduc Apr 29, 2024

scotthart left a comment

		# Some tests and quickstarts need to specify a location as "global".
		export GOOGLE_CLOUD_CPP_TEST_GLOBAL="global"

		connection = speech::MakeSpeechConnection(location);
		auto client = speech::SpeechClient(connection);

ci(speech): update quickstart to V2 demonstrating regional endpoints #13760

ci(speech): update quickstart to V2 demonstrating regional endpoints #13760

Conversation

scotthart commented Mar 11, 2024 • edited Loading

codecov bot commented Mar 11, 2024 • edited Loading

Codecov Report

scotthart left a comment

Choose a reason for hiding this comment

devbww Mar 11, 2024

Choose a reason for hiding this comment

devbww Mar 11, 2024

Choose a reason for hiding this comment

scotthart left a comment

Choose a reason for hiding this comment

devbww left a comment

Choose a reason for hiding this comment

scotthart left a comment

Choose a reason for hiding this comment

dbolduc Apr 26, 2024

Choose a reason for hiding this comment

dbolduc Apr 26, 2024

Choose a reason for hiding this comment

dbolduc Apr 29, 2024

Choose a reason for hiding this comment

scotthart left a comment

Choose a reason for hiding this comment

scotthart commented Mar 11, 2024 •

edited

Loading

codecov bot commented Mar 11, 2024 •

edited

Loading