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

feat: add Client.query_and_wait which directly returns a RowIterator of results #1722

Merged
merged 44 commits into from
Dec 8, 2023

Conversation

tswast
Copy link
Contributor

@tswast tswast commented Nov 15, 2023

Set the QUERY_PREVIEW_ENABLED=TRUE environment variable to use this with the new JOB_CREATION_OPTIONAL mode (currently in preview).

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Note: Currently in DRAFT mode as I realized that some of the necessary work to speed things up overlaps with the query(api_method="QUERY") work (#1723), so I'll work on modifying that first before moving on to the next step of a "jobless" API.

Towards #589 🦕

@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery API. labels Nov 15, 2023
timeout: Optional[float],
job_retry: retries.Retry,
) -> table.RowIterator:
"""Initiate a query using jobs.query with jobCreationMode=JOB_CREATION_REQUIRED.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be JOB_CREATION_OPTIONAL?
It seems that way based on the descriptions above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct. I'll update.

…or` of results

Set the `QUERY_PREVIEW_ENABLED=TRUE` environment variable to use this with the
new JOB_CREATION_OPTIONAL mode (currently in preview).
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Nov 20, 2023
@tswast tswast changed the base branch from main to issue589-RowIterator._is_completely_cached November 20, 2023 22:13
Base automatically changed from issue589-RowIterator._is_completely_cached to main November 21, 2023 15:13
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Nov 22, 2023
elif page_size is not None or max_results is not None:
request_body["maxResults"] = page_size or max_results

if os.getenv("QUERY_PREVIEW_ENABLED", "").casefold() == "true":

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tswast , in our previous discussion, I only suggested for this environment variable to use a list of project IDs for a more fine-grained control. We have discussed it with @shollyman , and for the Java client we decided to go with a boolean. I am fine either way, staying consistent with a boolean or doing a more fine-grained project id list (if required by python client usage).

This was being reset in some cases when
the rows were all available in the
first page of results.
if default_job_config is None:
return job_config

# anything that's not defined on the incoming
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a bit unclear. Can we clean this up a bit?

# should be filled in with the default
# the incoming therefore has precedence
#
# Note that _fill_from_default doesn't mutate the receiver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own edification, what do we mean by "receiver"?

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Copy link
Contributor

@chalmerlowe chalmerlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please address minor comments.

@tswast tswast enabled auto-merge (squash) December 8, 2023 22:14
@tswast tswast merged commit 89a647e into main Dec 8, 2023
21 of 22 checks passed
@tswast tswast deleted the issue589-query_and_wait branch December 8, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants