NotificationsYou must be signed in to change notification settings
Fork321
Star786

feat: search statistics#1616

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

chalmerlowe merged 56 commits intomainfromfeat-search-statistics

Sep 2, 2023

Merged

feat: search statistics#1616

chalmerlowe merged 56 commits intomainfromfeat-search-statistics

Sep 2, 2023

Conversation

Copy link

Collaborator

chalmerlowe commentedJul 20, 2023

Adds a feature to display search statistics in support of the following customer request:https://buganizer.corp.google.com/issues/290679162

Includes several classes to encapsulate the data found here:
https://cloud.google.com/bigquery/docs/reference/rest/v2/Job#searchstatistics

Also includes a function to access the data in the classes.

chalmerlowe added3 commits

July 19, 2023 14:58

experimental tweaks

b491836

feat: adds two search statistics classes and property

0cbb7f4

removes several personal debugging sentinels

1dbf528

chalmerlowe requested review froma team ascode owners

July 20, 2023 10:26

chalmerlowe requested a review fromfarhan0102

July 20, 2023 10:26

product-auto-labelbot added size: s

Pull request size is small.

api: bigqueryIssues related to the googleapis/python-bigquery API. labels

Jul 20, 2023

chalmerloweand others added5 commits

July 20, 2023 06:28

Merge branch 'main' into feat-search-statistics

9695712

🦉 Updates from OwlBot post-processor

fcf7012

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'feat-search-statistics' ofhttps://github.com/googleapi…

ae992bf

…s/python-bigquery into feat-search-statistics

🦉 Updates from OwlBot post-processor

3815371

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'feat-search-statistics' ofhttps://github.com/googleapi…

48086bd

…s/python-bigquery into feat-search-statistics

shollyman requested review fromshollyman and removed request forfarhan0102

July 20, 2023 15:57

shollyman reviewed

Jul 20, 2023

View reviewed changes

Copy link

Contributor

shollyman left a comment

Choose a reason for hiding this comment

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

Good start on the basic structure. This is an output-only stat so no need for a setter, but we'll need all the unit testing and a similar IT test to java using a query that invokes thesearch() sql function.

google/cloud/bigquery/job/query.py Outdated

		"""docstring"""

		stats=self._job_statistics().get("searchStatistics")
		returnSearchStats.from_api_repr(stats)

Copy link

Contributor

shollymanJul 20, 2023

Choose a reason for hiding this comment

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

need to handle the not-present case, e.g.if stats is not None:

google/cloud/bigquery/job/query.py Outdated


		@classmethod
		deffrom_api_repr(cls,stats:Dict[str,Any]):
		mode=stats.get("indexUsageMode","INDEX_USAGE_MODE_UNSPECIFIED")

Copy link

Contributor

shollymanJul 20, 2023

Choose a reason for hiding this comment

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

Generally, we want to avoid the library injecting values not present in the API response, so returning the default value here may not be more desirable than empty/none.

adds tests

5b14d5d

product-auto-labelbot added size: mPull request size is medium. and removed size: sPull request size is small. labels

Aug 9, 2023

gcf-owl-botbotand others added10 commits

August 9, 2023 18:36

🦉 Updates from OwlBot post-processor

9689a71

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

cleans up conflict

6af7d6f

adds comment

53437f2

adds some type hints, adds a test for SearchReasons

4fd77ba

cleans up some comments

bb2b52c

Merge branch 'main' into feat-search-statistics

e728883

🦉 Updates from OwlBot post-processor

966ddb1

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'feat-search-statistics' ofhttps://github.com/googleapi…

e9fca23

…s/python-bigquery into feat-search-statistics

🦉 Updates from OwlBot post-processor

ba2eb65

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'feat-search-statistics' ofhttps://github.com/googleapi…

a0a0e5b

…s/python-bigquery into feat-search-statistics

chalmerlowe commented

Aug 11, 2023

View reviewed changes

tests/unit/job/test_query_stats.py OutdatedShow resolvedHide resolved

chalmerlowe added kokoro:run

Add this label to force Kokoro to re-run the tests.

kokoro:force-runAdd this label to force Kokoro to re-run the tests. labels

Aug 23, 2023

yoshi-kokoro removed kokoro:run

Add this label to force Kokoro to re-run the tests.

kokoro:force-runAdd this label to force Kokoro to re-run the tests. labels

Aug 23, 2023

chalmerloweand others added11 commits

August 23, 2023 15:11

Merge branch 'main' into feat-search-statistics

0423fdc

experiments with some tests that are failing

496ab6d

🦉 Updates from OwlBot post-processor

392db37

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

resolves merge conflict

a30f23c

resolves merge conflict

c1dfc35

resolve conflicts

5a2f268

resolve merge conflicts

744d932

🦉 Updates from OwlBot post-processor

81f8aab

Seehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

updates due to faulty confict resolution

861eb98

adds docstrings to two classes

20ec9e3

corrects formatting

5659dd3

chalmerlowe commented

Aug 24, 2023

View reviewed changes

tests/unit/job/test_query_stats.py OutdatedShow resolvedHide resolved

chalmerlowe added2 commits

August 24, 2023 12:40

Update tests/unit/job/test_query_stats.py

dd1b749

Update tests/unit/job/test_query_stats.py

e056787

shollyman reviewed

Aug 24, 2023

View reviewed changes

Copy link

Contributor

shollyman left a comment

Choose a reason for hiding this comment

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

After some more reading, I think the attribute approach is sufficient vs explicit property annotations.

Main concern is about attribute defaults. Typically None is better when users are trying to disambiguate between presence of a value vs the empty value, but happy to talk about this more. Swapping the default impacts typing, unit tests, etc.