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.

Sign up for GitHub

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

Jump to bottom

fix: add df snapshots lookup for read_gbq #229

Merged
merged 18 commits into from
Nov 29, 2023
Merged

fix: add df snapshots lookup for read_gbq #229

merged 18 commits into from
Nov 29, 2023

Conversation

ashleyxuu
Copy link
Contributor

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)

Fixes interbal issue 312493159 🦕

@ashleyxuu ashleyxuu requested review from a team as code owners November 23, 2023 00:14
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Nov 23, 2023
@ashleyxuu ashleyxuu requested review from TrevorBergeron and tswast and removed request for stevewalker-de November 23, 2023 00:15
@ashleyxuu ashleyxuu changed the title fix: df snapshots lookup fix: add df snapshots lookup for read_gbq Nov 23, 2023
jiaxunwu
jiaxunwu reviewed Nov 24, 2023
View reviewed changes
bigframes/session/__init__.py Outdated Show resolved Hide resolved
bigframes/session/__init__.py Show resolved Hide resolved
tswast
tswast reviewed Nov 28, 2023
View reviewed changes
bigframes/pandas/__init__.py Outdated Show resolved Hide resolved
bigframes/session/__init__.py
@@ -246,6 +247,7 @@ def read_gbq(
index_col: Iterable[str] | str = (),
col_order: Iterable[str] = (),
max_results: Optional[int] = None,
use_cache: bool = True,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we pass this to _read_gbq_query too, please? It can be used to populate this property:

https://cloud.google.com/python/docs/reference/bigquery/latest/google.cloud.bigquery.job.QueryJobConfig#google_cloud_bigquery_job_QueryJobConfig_use_query_cache

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unresolving. You didn't populate the QueryJobConfig.use_query_cache property as I requested.

bigframes/session/__init__.py Show resolved Hide resolved
tswast
tswast requested changes Nov 28, 2023
View reviewed changes
bigframes/session/__init__.py Outdated Show resolved Hide resolved
bigframes/session/__init__.py
@@ -246,6 +247,7 @@ def read_gbq(
index_col: Iterable[str] | str = (),
col_order: Iterable[str] = (),
max_results: Optional[int] = None,
use_cache: bool = True,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unresolving. You didn't populate the QueryJobConfig.use_query_cache property as I requested.

bigframes/session/__init__.py Outdated Show resolved Hide resolved
tswast
tswast approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Thanks!

@ashleyxuu ashleyxuu added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 29, 2023
@ashleyxuu ashleyxuu added the automerge Merge the pull request once unit tests and other checks pass. label Nov 29, 2023
@bigframes-bot bigframes-bot removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 29, 2023
@gcf-merge-on-green GCF Merge on Green
Copy link

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Nov 29, 2023
@ashleyxuu ashleyxuu merged commit d0d9b84 into main Nov 29, 2023
14 of 15 checks passed
@ashleyxuu ashleyxuu deleted the ashleyxu-df-snapshots-lookup branch November 29, 2023 17:15
@release-please release-please bot mentioned this pull request Nov 29, 2023
Merged
Genesis929 pushed a commit that referenced this pull request Dec 12, 2023
@ashleyxuu @Genesis929
fix: add df snapshots lookup for read_gbq (#229)
f1571fa
@tswast tswast mentioned this pull request Mar 14, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiaxunwu jiaxunwu jiaxunwu left review comments

@tswast tswast tswast approved these changes

@TrevorBergeron TrevorBergeron Awaiting requested review from TrevorBergeron

Assignees
No one assigned
Labels
api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashleyxuu @tswast @jiaxunwu @bigframes-bot