Update BQ Query parameter reference

[roscoejp]

Update BigQuery Query parameter reference to indicate that timeout_ms sets timeout on the request, not on the query. Should address #3317

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[roscoejp]

I signed it!

[googlebot]

CLAs look good, thanks!

[tseaver]

The docs for timeoutMs actually say:

[Optional] How long to wait for the query to complete, in milliseconds, before the request times out and returns. Note that this is only a timeout for the request, not the query. If the query takes longer to run than the timeout value, the call returns without any results and with the 'jobComplete' flag set to false. You can call GetQueryResults() to wait for the query to complete and read the results. The default value is 10000 milliseconds (10 seconds).

Which makes your change incorrect: the timeout occurs on the server side, not in the client.

[lukesneeringer]

@tseaver Your point is orthogonal to @roscoejp's point. The timeout is the request timeout (for that specific HTTP/GRPC request) as opposed to the query timeout. The query continues to run. Note the penultimate sentence:

You can call `GetQueryResults() to wait for the query to complete and read the results.

That is very much a request timeout, as opposed to a query timeout.

It is true that it is the server that drops the connection, but that distinction is not relevant for most (any?) users. They do not care which side severed the connection.

[tseaver]

@lukesneeringer Wouldn't it be better to include more of the semantics from the API docs than go with the language in this PR? Otherwise we end up introducing potential confusion with other client-driven timeouts.

[roscoejp]

I was originally trying to keep this short to keep it in line with the other param references. I changed it from 'query timeout' to 'request timeout' in order to bring it more in line with the API docs - specifically:

Note that this is only a timeout for the request, not the query

The current reference is actually more confusing than this change because it specifically contradicts the API docs. It also seems unnecessary to make the param reference more verbose since there's already a link to the very detailed API doc in the code.

[lukesneeringer]

How about we include the exact language that the API does? That way we can wash our hands of what the exact language should be.

[roscoejp]

Pushed the whole timeout_ms reference. Lemme know if there are any other changes you'd like made to this reference.

[tseaver]

@roscoejp Thanks for the fix, and your willingness to follow up!

[roscoejp]

No problem. Committing to the repos isn't part of my normal routine - but we've had a few cases come in with customers getting confused about this (particular) issue so this seemed like the fastest way to get it fixed.

Thanks for your time.