feat: async execute query client #1011

kboroszko · 2024-08-05T14:28:24Z

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)

Description

This PR introduces support for the newly added executeQuery RPC in the Bigtable async client.

It handles executing parameterized queries and receiving streaming responses through an asynchronous iterator.
Most of the logic is located in google.cloud.bigtable.data.execute_query.
Example usage is in samples/snippets/data_client/data_client_snippets_async.py

Co-authored-by: Mateusz Walkiewicz <[email protected]>

snippet-bot · 2024-08-05T14:28:31Z

Here is the summary of changes.

You are about to add 1 region tag.

samples/snippets/data_client/data_client_snippets_async.py:257, tag bigtable_async_execute_query

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

daniel-sanche

Added a couple requests for missing docstrings, etc. But overall LGTM once the tests pass

daniel-sanche · 2024-08-05T16:24:14Z

google/cloud/bigtable/data/exceptions.py

+
+
+class ParameterTypeInferenceFailed(ValueError):
+    pass


nit: it would be good to have a docstring here

daniel-sanche · 2024-08-05T16:24:42Z

google/cloud/bigtable/data/execute_query/_async/execute_query_iterator.py

+)
+
+
+class ExecuteQueryIteratorAsync:


daniel-sanche · 2024-08-05T16:25:13Z

google/cloud/bigtable/data/execute_query/_async/execute_query_iterator.py

+    def __aiter__(self):
+        return self
+
+    async def metadata(self) -> Optional[Metadata]:


please add docstrings on all of these methods

daniel-sanche · 2024-08-05T16:25:45Z

google/cloud/bigtable/data/execute_query/_parameters_formatting.py

+    params: Optional[Dict[str, ExecuteQueryValueType]],
+    parameter_types: Optional[Dict[str, SqlType.Type]],
+):
+    if not params:


docstrings on these would also be helpful for future maintenance

It doesn't need to be too complex for the internal functions, but some context would be useful

daniel-sanche · 2024-08-05T16:26:03Z

google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py

+
+
+def _parse_array_type(value: PBValue, metadata_type: SqlType.Array) -> list:
+    return list(


docstrings here too

daniel-sanche · 2024-08-05T16:28:32Z

google/cloud/bigtable_v2/__init__.py

@@ -122,6 +122,8 @@
    "RowSet",
    "SampleRowKeysRequest",
    "SampleRowKeysResponse",
+    "ExecuteQueryRequest",
+    "ExecuteQueryResponse",


This file is autogenerated and doesn't need to be touched (These are already included in lines 87/88)

daniel-sanche · 2024-08-05T16:30:40Z

tests/unit/data/test_byte_cursor.py

@@ -0,0 +1,149 @@
+# Copyright 2024 Google LLC


Previously, the test files mirrored the structure of the source files. It might make sense to throw these into an execute_query directory?

daniel-sanche · 2024-08-05T23:13:40Z

FYI, you can ignore the failing samples tests. There's a quota issue in the samples project we're looking in to

daniel-sanche · 2024-08-07T16:29:42Z

google/cloud/bigtable/data/execute_query/metadata.py

@@ -212,13 +221,13 @@ class Timestamp(Type):
            DatetimeWithNanoseconds,
        )

-        def _to_value_pb_dict(self, value: Any) -> dict:
+        def _to_value_pb_dict(self, value: Any) -> Any:


It's not too important, but do we have to use Any for these? Does dict[str, Any] not work?

google/cloud/bigtable/data/execute_query/values.py

daniel-sanche · 2024-08-08T16:56:03Z

samples/snippets/data_client/data_client_snippets_async.py

+            query = (
+                "SELECT _key, os_build, connected_cell, connected_wifi "
+                f"from {table_id} WHERE _key=@row_key"
+            )


I know I said to ignore the samples tests, but it looks like there's actually an issue here:

google.api_core.exceptions.InvalidArgument: 400 Syntax error: Missing whitespace between literal and alias [at 1:87]

Also, there's a snippet lint issue that can be addressed using

cd samples/snippets/data_client nox -s blacken

To fix the invalid argument do: f"from `{table_id}` WHERE _key=@row_key" (wraps the table name in backticks)

Daniel pointed out the column names here aren't working either. All of those are qualifiers.

We should update the first line of the query to SELECT _key, stats_summary['os_build'], stats_summary['connected_cell'], stats_summary['connected_wifi']

daniel-sanche

blocking until sample is addressed

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

daniel-sanche

LGTM

kboroszko and others added 4 commits July 31, 2024 10:40

feat: ByteCursor and ProtoRowsReader

64a2496

feat: async client

92cda65

Co-authored-by: Mateusz Walkiewicz <[email protected]>

protbufs update

9554eb2

fix: refactor execute query related files (googleapis#34)

88e0492

kboroszko requested review from a team as code owners August 5, 2024 14:28

kboroszko requested a review from msampathkumar August 5, 2024 14:28

blunderbuss-gcf bot assigned triplequark Aug 5, 2024

daniel-sanche approved these changes Aug 5, 2024

View reviewed changes

product-auto-label bot added the api: bigtable Issues related to the googleapis/python-bigtable API. label Aug 6, 2024

pr fixes 1

4196fff

triplequark assigned daniel-sanche and unassigned triplequark Aug 6, 2024

daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024

yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024

daniel-sanche added the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 6, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 6, 2024

mypy changes

712edef

product-auto-label bot added the size: xl Pull request size is extra large. label Aug 7, 2024

fix tests

6d536e6

daniel-sanche reviewed Aug 7, 2024

View reviewed changes

Struct __eq__ fixes

73299af

jackdingilian added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 8, 2024

yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 8, 2024

jackdingilian reviewed Aug 8, 2024

View reviewed changes

google/cloud/bigtable/data/execute_query/values.py Outdated Show resolved Hide resolved

daniel-sanche reviewed Aug 8, 2024

View reviewed changes

daniel-sanche requested changes Aug 8, 2024

View reviewed changes

fixed prints, samples, typing

a9263f2

daniel-sanche added kokoro:run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 8, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 8, 2024

yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 8, 2024

🦉 Updates from OwlBot post-processor

92a7c38

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

daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 8, 2024

yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 8, 2024

daniel-sanche approved these changes Aug 8, 2024

View reviewed changes

jackdingilian approved these changes Aug 8, 2024

View reviewed changes

daniel-sanche merged commit 45bc8c4 into googleapis:main Aug 8, 2024
29 of 30 checks passed

release-please bot mentioned this pull request Aug 5, 2024

chore(main): release 2.26.0 #1006

Merged



		def _parse_array_type(value: PBValue, metadata_type: SqlType.Array) -> list:
		return list(

feat: async execute query client #1011

feat: async execute query client #1011

Uh oh!

Conversation

kboroszko commented Aug 5, 2024

Description

Uh oh!

snippet-bot bot commented Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-sanche left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche commented Aug 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

snippet-bot bot commented Aug 5, 2024 •

edited

Loading

daniel-sanche left a comment •

edited

Loading

daniel-sanche Aug 5, 2024 •

edited

Loading