Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

refactor: cache table metadata alongside snapshot time #636

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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

tswast
Copy link
Collaborator

@tswast tswast commented Apr 24, 2024

This ensures the cached primary_keys is more likely to be correct, in case the user called ALTER TABLE after we originally cached the snapshot time.

Towards internal issue 335727141 and #631
馃

This ensures the cached `primary_keys` is more likely to be
correct, in case the user called ALTER TABLE after we originally
cached the snapshot time.
@tswast tswast requested review from a team as code owners April 24, 2024 20:14
@tswast tswast requested a review from chelsea-lin April 24, 2024 20:14
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Apr 24, 2024
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. label Apr 24, 2024
Copy link
Contributor

@chelsea-lin chelsea-lin left a comment

Choose a reason for hiding this comment

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

LGTM

cache: Dict[bigquery.TableReference, Tuple[datetime.datetime, bigquery.Table]],
use_cache: bool = True,
) -> Tuple[datetime.datetime, bigquery.Table]:
cached_table = cache.get(table_ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if use_cache=False, we can avoid looking through the cache, though the cache won't be too large and the looking is not too expensive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good observation!

Python does dictionary lookups all the time, even for accessing attributes of objects, so I'm not too worried about it. Will leave as-is to avoid the extra nesting.

@tswast tswast merged commit 3acc494 into main Apr 25, 2024
@tswast tswast deleted the b335727141-snapshot-save-metadata branch April 25, 2024 14:14
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-dataframes API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.