feat: add test proxy #836

daniel-sanche · 2023-07-18T00:25:54Z

Adds test proxy implementation, allowing us to run language-agnostic conformance tests against the new client

Also adds support for connecting data client to an emulator

This is an update from #747

TODO:

add conformance tests to presubmit tests

test_proxy/README.md

test_proxy/protos/v2_test_proxy_pb2.py

google/cloud/bigtable/data/_async/client.py

liujiongxin · 2023-07-25T18:55:48Z

test_proxy/client_handler.py

+        # look for code in subexceptions
+        for subexc in result["subexceptions"]:
+            if subexc.get("code", None):
+                result["code"] = subexc["code"]


This will record the last exception's code. Is it intended? If so, does it correspond to the first exception occurrence (i.e., the head/root of the exception chain)?

Yes, we make use of exception groups so that we can expose a set of retried errors together with the final error. The exceptions are wrapped in a bigtable.RetryException. The final exception in the group should be the most relevant one

More details here: #825

liujiongxin · 2023-07-25T21:43:51Z

test_proxy/client_handler.py

+                true_mutations.append(Mutation._from_dict(mut_dict))
+            except ValueError:
+                # invalid mutation type. Conformance test may be sending generic empty request
+                true_mutations.append(SetCell("", "", "", -1))


Does it have real effect on the client? I wonder if no-op here can also work?

It's been a while since I looked at this, but I think this is the one spot where I couldn't get a test to pass without modifications to the test code

I think the library will raise an exception if I try to create an empty SetCell mutation, but I believe the tests expect it. I was going to suggest we change the example mutation in the conformance tests to one with valid data

I see. You are right.. But somehow they other clients seems to be fine with the empty set. You can keep the line as is. I will make the test change later and let you know when it's done.

liujiongxin · 2023-07-25T21:46:51Z

test_proxy/client_handler.py

+        project_id=None,
+        instance_id=None,
+        app_profile_id=None,
+        per_operation_timeout=None,


what's the expected unit/format? The implementation seems to imply that it's integer?

for the timeouts? They are float or None, but python accepts ints as floats

Does it mean seconds or milliseconds?

it's in seconds. It's better documented here:

python-bigtable/google/cloud/bigtable/data/_async/client.py

Line 574 in aa760b2

- operation_timeout: the time budget for the entire operation, in seconds.

Does that match the data sent from the tests?

liujiongxin · 2023-07-25T21:51:26Z

test_proxy/client_handler_legacy.py

How much do you want to support legacy client? I only see 3 APIs are implemented, what's the future plan (including deprecation)?

We will support them side-by-side for the medium term, with plans to eventually deprecate the old client. For now, my attention has been focused on the new data client. But it would be good to get test parity at some point

test_proxy/grpc_handler.py

test_proxy/test_proxy.py

liujiongxin · 2023-07-26T00:12:28Z

test_proxy/noxfile.py

+        session.run("go", "test", "-v", f"-proxy_addr=:{PROXY_SERVER_PORT}")
+
+def default(session):
+    """Run the performance test suite."""


The implementation is to run the proxy?

This may have been a copy/paste error. Fixed

liujiongxin · 2023-07-26T00:21:18Z

test_proxy/test_proxy.py

+        ),
+    )
+    proxy.start()
+    client_handler_process(request_q, response_queue_pool, use_legacy_client)


How is it terminated? For java client testing, we use this: https://github.com/googleapis/java-bigtable/blob/main/.kokoro/conformance.sh#L59

Good point. It currently isn't automatically terminated, but I will need to add that for running automated tests.

I may just do this as part of the kokoro script when I add this to kokoro. Is it alright if we make that a separate PR?

liujiongxin · 2023-08-16T17:40:54Z

test_proxy/client_handler.py

+        project_id=None,
+        instance_id=None,
+        app_profile_id=None,
+        per_operation_timeout=None,


Does it mean seconds or milliseconds?

liujiongxin · 2023-08-16T17:46:07Z

test_proxy/client_handler.py

+                true_mutations.append(Mutation._from_dict(mut_dict))
+            except ValueError:
+                # invalid mutation type. Conformance test may be sending generic empty request
+                true_mutations.append(SetCell("", "", "", -1))


I see. You are right.. But somehow they other clients seems to be fine with the empty set. You can keep the line as is. I will make the test change later and let you know when it's done.

liujiongxin · 2023-08-16T17:47:32Z

test_proxy/test_proxy.py

+        ),
+    )
+    proxy.start()
+    client_handler_process(request_q, response_queue_pool, use_legacy_client)


* feat: add new v3.0.0 API skeleton (#745) * feat: improve rows filters (#751) * feat: read rows query model class (#752) * feat: implement row and cell model classes (#753) * feat: add pooled grpc transport (#748) * feat: implement read_rows (#762) * feat: implement mutate rows (#769) * feat: literal value filter (#767) * feat: row_exists and read_row (#778) * feat: read_modify_write and check_and_mutate_row (#780) * feat: sharded read rows (#766) * feat: ping and warm with metadata (#810) * feat: mutate rows batching (#770) * chore: restructure module paths (#816) * feat: improve timeout structure (#819) * fix: api errors apply to all bulk mutations * chore: reduce public api surface (#820) * feat: improve error group tracebacks on < py11 (#825) * feat: optimize read_rows (#852) * chore: add user agent suffix (#842) * feat: optimize retries (#854) * feat: add test proxy (#836) * chore(tests): add conformance tests to CI for v3 (#870) * chore(tests): turn off fast fail for conformance tets (#882) * feat: add TABLE_DEFAULTS enum for table method arguments (#880) * fix: pass None for retry in gapic calls (#881) * feat: replace internal dictionaries with protos in gapic calls (#875) * chore: optimize gapic calls (#863) * feat: expose retryable error codes to users (#879) * chore: update api_core submodule (#897) * chore: merge main into experimental_v3 (#900) * chore: pin conformance tests to v0.0.2 (#903) * fix: bulk mutation eventual success (#909) --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

daniel-sanche added 9 commits July 17, 2023 15:35

support emulator in data client

aa1e40e

added test proxy files

b059139

cleaned up noxfile

3ed2168

moved protos to subdir

5199839

close client

4e14bf3

moved handlers into subdir

cbb95c9

reverted close

f43aac1

removed user agent

91fc1e6

removed go submodule

06e5276

daniel-sanche requested review from a team as code owners July 18, 2023 00:25

product-auto-label bot added size: xl Pull request size is extra large. api: bigtable Issues related to the googleapis/python-bigtable API. labels Jul 18, 2023

liujiongxin reviewed Jul 18, 2023

View reviewed changes

test_proxy/README.md Outdated Show resolved Hide resolved

liujiongxin reviewed Jul 18, 2023

View reviewed changes

test_proxy/protos/v2_test_proxy_pb2.py Outdated Show resolved Hide resolved

daniel-sanche added 3 commits July 20, 2023 14:24

fixied typo

62b8e48

removed unneeded files

237e051

removed duplicate client handler legacy

868ff2e

daniel-sanche mentioned this pull request Jul 20, 2023

[DRAFT] chore: benchmark tests #840

Closed

liujiongxin reviewed Jul 26, 2023

View reviewed changes

daniel-sanche and others added 3 commits August 16, 2023 10:35

Merge branch 'v3' into test_proxy2

21a5077

addressed PR comments

02f0c09

ran blacken

456caba

liujiongxin approved these changes Aug 16, 2023

View reviewed changes

daniel-sanche added 6 commits August 16, 2023 11:32

Merge branch 'v3' into test_proxy2

f3627c1

fix method name

bcc02d7

added missing import

5e7c156

added conformance tests to kokoro

604d3d8

added conformance to nox sessions

14f359d

Revert unwwanted noxfile changes

858c57d

added missing run_tests file

36a3153

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

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

daniel-sanche added 2 commits August 17, 2023 12:56

changed conformance test kokoro configs

07b39b1

ran blacken

5d90478

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

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

daniel-sanche added 10 commits August 17, 2023 14:05

install golang for conformance tests

b69da5a

update before attempting install

df3ea47

changed go install method

8dcd444

moved go installation to run_tests

94a8684

fixed failing conformance tests

72b8d1b

Merge branch 'v3' into test_proxy2

8496211

fixed read rows test error

71ba0ea

fixed conformance test errors

94e98db

download go locally instead of installing to system

320d157

fixed lint issue

5064870

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

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

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

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

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

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

daniel-sanche merged commit 8708a25 into googleapis:v3 Aug 18, 2023
4 checks passed

daniel-sanche mentioned this pull request Oct 18, 2023

chore(tests): add conformance tests #872

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add test proxy #836

feat: add test proxy #836

daniel-sanche commented Jul 18, 2023

liujiongxin Jul 25, 2023

daniel-sanche Aug 16, 2023

liujiongxin Jul 25, 2023

daniel-sanche Aug 16, 2023

liujiongxin Aug 16, 2023

liujiongxin Jul 25, 2023

daniel-sanche Aug 16, 2023

liujiongxin Aug 16, 2023

daniel-sanche Aug 16, 2023

liujiongxin Jul 25, 2023

daniel-sanche Aug 16, 2023

liujiongxin Jul 26, 2023

daniel-sanche Aug 16, 2023

liujiongxin Jul 26, 2023

daniel-sanche Aug 16, 2023 •

edited

Loading

liujiongxin Aug 16, 2023

liujiongxin Aug 16, 2023

liujiongxin Aug 16, 2023

liujiongxin Aug 16, 2023

feat: add test proxy #836

feat: add test proxy #836

Conversation

daniel-sanche commented Jul 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-sanche Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-sanche Aug 16, 2023 •

edited

Loading