From 55186453239fc5e358a79e82f4cc3bb5488fe9e5 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 6 Sep 2023 22:59:58 +0000 Subject: [PATCH 01/23] feat: optimize _GapicCallable --- google/api_core/gapic_v1/method.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 0c1624a3..9370ccc0 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -81,23 +81,26 @@ class _GapicCallable(object): def __init__(self, target, retry, timeout, metadata=None): self._target = target self._retry = retry + if isinstance(timeout, (int, float)): + timeout = TimeToDeadlineTimeout(timeout=timeout) self._timeout = timeout self._metadata = metadata def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): """Invoke the low-level RPC with retry, timeout, and metadata.""" + wrapped_func = self._target if retry is DEFAULT: retry = self._retry + if retry is not None: + wrapped_func = retry(wrapped_func) if timeout is DEFAULT: timeout = self._timeout - - if isinstance(timeout, (int, float)): + elif isinstance(timeout, (int, float)): timeout = TimeToDeadlineTimeout(timeout=timeout) - - # Apply all applicable decorators. - wrapped_func = _apply_decorators(self._target, [retry, timeout]) + if timeout is not None: + wrapped_func = timeout(wrapped_func) # Add the user agent metadata to the call. if self._metadata is not None: From 0cc03bdacaa7032b6edcb33ebd330703e2f5218e Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 7 Sep 2023 23:49:51 +0000 Subject: [PATCH 02/23] cleaned up metadata lines --- google/api_core/gapic_v1/method.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 9370ccc0..ad0a4801 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -109,9 +109,7 @@ def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): # as not specified. if metadata is None: metadata = [] - metadata = list(metadata) - metadata.extend(self._metadata) - kwargs["metadata"] = metadata + kwargs["metadata"] = (*self._metadata, *metadata) return wrapped_func(*args, **kwargs) From c97a6365028f3f04d20f26aa1cc0e3131164f53e Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 7 Sep 2023 23:54:14 +0000 Subject: [PATCH 03/23] chore: avoid type checks in error wrapper --- google/api_core/grpc_helpers_async.py | 29 ++++++++++----------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/google/api_core/grpc_helpers_async.py b/google/api_core/grpc_helpers_async.py index 5a5bf2a6..dd422d02 100644 --- a/google/api_core/grpc_helpers_async.py +++ b/google/api_core/grpc_helpers_async.py @@ -149,8 +149,6 @@ class _WrappedStreamStreamCall( def _wrap_unary_errors(callable_): """Map errors for Unary-Unary async callables.""" - grpc_helpers._patch_callable_name(callable_) - @functools.wraps(callable_) def error_remapped_callable(*args, **kwargs): call = callable_(*args, **kwargs) @@ -159,25 +157,13 @@ def error_remapped_callable(*args, **kwargs): return error_remapped_callable -def _wrap_stream_errors(callable_): +def _wrap_stream_errors(callable_, wrapper_type): """Map errors for streaming RPC async callables.""" - grpc_helpers._patch_callable_name(callable_) - @functools.wraps(callable_) async def error_remapped_callable(*args, **kwargs): call = callable_(*args, **kwargs) - - if isinstance(call, aio.UnaryStreamCall): - call = _WrappedUnaryStreamCall().with_call(call) - elif isinstance(call, aio.StreamUnaryCall): - call = _WrappedStreamUnaryCall().with_call(call) - elif isinstance(call, aio.StreamStreamCall): - call = _WrappedStreamStreamCall().with_call(call) - else: - raise TypeError("Unexpected type of call %s" % type(call)) - await call.wait_for_connection() - return call + return wrapper_type().with_call(call) return error_remapped_callable @@ -197,10 +183,17 @@ def wrap_errors(callable_): Returns: Callable: The wrapped gRPC callable. """ + grpc_helpers._patch_callable_name(callable_) if isinstance(callable_, aio.UnaryUnaryMultiCallable): - return _wrap_unary_errors(callable_) else: - return _wrap_stream_errors(callable_) + if isinstance(callable_, aio.UnaryStreamMultiCallable): + return _wrap_stream_errors(callable_, _WrappedUnaryStreamCall) + elif isinstance(callable_, aio.StreamUnaryMultiCallable): + return _wrap_stream_errors(callable_, _WrappedStreamUnaryCall) + elif isinstance(callable_, aio.StreamStreamMultiCallable): + return _wrap_stream_errors(callable_, _WrappedStreamStreamCall) + else: + raise TypeError("Unexpected type of callable: {}".format(type(callable_))) def create_channel( From b453df4c5e40cd0612b64705f171e86d2fda09c1 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 7 Sep 2023 16:56:08 -0700 Subject: [PATCH 04/23] Revert "chore: avoid type checks in error wrapper" This reverts commit c97a6365028f3f04d20f26aa1cc0e3131164f53e. --- google/api_core/grpc_helpers_async.py | 29 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/google/api_core/grpc_helpers_async.py b/google/api_core/grpc_helpers_async.py index dd422d02..5a5bf2a6 100644 --- a/google/api_core/grpc_helpers_async.py +++ b/google/api_core/grpc_helpers_async.py @@ -149,6 +149,8 @@ class _WrappedStreamStreamCall( def _wrap_unary_errors(callable_): """Map errors for Unary-Unary async callables.""" + grpc_helpers._patch_callable_name(callable_) + @functools.wraps(callable_) def error_remapped_callable(*args, **kwargs): call = callable_(*args, **kwargs) @@ -157,13 +159,25 @@ def error_remapped_callable(*args, **kwargs): return error_remapped_callable -def _wrap_stream_errors(callable_, wrapper_type): +def _wrap_stream_errors(callable_): """Map errors for streaming RPC async callables.""" + grpc_helpers._patch_callable_name(callable_) + @functools.wraps(callable_) async def error_remapped_callable(*args, **kwargs): call = callable_(*args, **kwargs) + + if isinstance(call, aio.UnaryStreamCall): + call = _WrappedUnaryStreamCall().with_call(call) + elif isinstance(call, aio.StreamUnaryCall): + call = _WrappedStreamUnaryCall().with_call(call) + elif isinstance(call, aio.StreamStreamCall): + call = _WrappedStreamStreamCall().with_call(call) + else: + raise TypeError("Unexpected type of call %s" % type(call)) + await call.wait_for_connection() - return wrapper_type().with_call(call) + return call return error_remapped_callable @@ -183,17 +197,10 @@ def wrap_errors(callable_): Returns: Callable: The wrapped gRPC callable. """ - grpc_helpers._patch_callable_name(callable_) if isinstance(callable_, aio.UnaryUnaryMultiCallable): + return _wrap_unary_errors(callable_) else: - if isinstance(callable_, aio.UnaryStreamMultiCallable): - return _wrap_stream_errors(callable_, _WrappedUnaryStreamCall) - elif isinstance(callable_, aio.StreamUnaryMultiCallable): - return _wrap_stream_errors(callable_, _WrappedStreamUnaryCall) - elif isinstance(callable_, aio.StreamStreamMultiCallable): - return _wrap_stream_errors(callable_, _WrappedStreamStreamCall) - else: - raise TypeError("Unexpected type of callable: {}".format(type(callable_))) + return _wrap_stream_errors(callable_) def create_channel( From 2f7acffa0481b8b15d5076ddc3172574d2896741 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Sep 2023 00:06:39 +0000 Subject: [PATCH 05/23] add default wrapped function --- google/api_core/gapic_v1/method.py | 40 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index ad0a4801..ed245d69 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -85,22 +85,14 @@ def __init__(self, target, retry, timeout, metadata=None): timeout = TimeToDeadlineTimeout(timeout=timeout) self._timeout = timeout self._metadata = metadata - - def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): - """Invoke the low-level RPC with retry, timeout, and metadata.""" - - wrapped_func = self._target - if retry is DEFAULT: - retry = self._retry + self._default_wrapped_fn = target if retry is not None: - wrapped_func = retry(wrapped_func) - - if timeout is DEFAULT: - timeout = self._timeout - elif isinstance(timeout, (int, float)): - timeout = TimeToDeadlineTimeout(timeout=timeout) + self._default_wrapped_fn = retry(target) if timeout is not None: - wrapped_func = timeout(wrapped_func) + self._default_wrapped_fn = timeout(target) + + def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): + """Invoke the low-level RPC with retry, timeout, and metadata.""" # Add the user agent metadata to the call. if self._metadata is not None: @@ -110,7 +102,25 @@ def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): if metadata is None: metadata = [] kwargs["metadata"] = (*self._metadata, *metadata) - + + if retry is DEFAULT and timeout is DEFAULT: + # if default timeout and retry is passed, use cached callable + wrapped_func = self._default_wrapped_fn + else: + # otherwise, construct wrapped function + wrapped_func = self._target + if retry is DEFAULT: + retry = self._retry + if retry is not None: + wrapped_func = retry(wrapped_func) + + if timeout is DEFAULT: + timeout = self._timeout + elif isinstance(timeout, (int, float)): + timeout = TimeToDeadlineTimeout(timeout=timeout) + if timeout is not None: + wrapped_func = timeout(wrapped_func) + return wrapped_func(*args, **kwargs) From 31f0b4ee71ff0bcd70810ecdefa12575258604b5 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Sep 2023 01:10:43 +0000 Subject: [PATCH 06/23] fixed decorator order --- google/api_core/gapic_v1/method.py | 37 +++++++++++------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index ed245d69..7ae6258a 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -85,14 +85,23 @@ def __init__(self, target, retry, timeout, metadata=None): timeout = TimeToDeadlineTimeout(timeout=timeout) self._timeout = timeout self._metadata = metadata - self._default_wrapped_fn = target - if retry is not None: - self._default_wrapped_fn = retry(target) - if timeout is not None: - self._default_wrapped_fn = timeout(target) def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): """Invoke the low-level RPC with retry, timeout, and metadata.""" + + # onstruct wrapped function + wrapped_func = self._target + if timeout is DEFAULT: + timeout = self._timeout + elif isinstance(timeout, (int, float)): + timeout = TimeToDeadlineTimeout(timeout=timeout) + if timeout is not None: + wrapped_func = timeout(wrapped_func) + + if retry is DEFAULT: + retry = self._retry + if retry is not None: + wrapped_func = retry(wrapped_func) # Add the user agent metadata to the call. if self._metadata is not None: @@ -102,24 +111,6 @@ def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): if metadata is None: metadata = [] kwargs["metadata"] = (*self._metadata, *metadata) - - if retry is DEFAULT and timeout is DEFAULT: - # if default timeout and retry is passed, use cached callable - wrapped_func = self._default_wrapped_fn - else: - # otherwise, construct wrapped function - wrapped_func = self._target - if retry is DEFAULT: - retry = self._retry - if retry is not None: - wrapped_func = retry(wrapped_func) - - if timeout is DEFAULT: - timeout = self._timeout - elif isinstance(timeout, (int, float)): - timeout = TimeToDeadlineTimeout(timeout=timeout) - if timeout is not None: - wrapped_func = timeout(wrapped_func) return wrapped_func(*args, **kwargs) From b92328c54ba04b4af9d1f9374ef07f03f8c0c01b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Sep 2023 01:11:23 +0000 Subject: [PATCH 07/23] fixed spacing --- google/api_core/gapic_v1/method.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 7ae6258a..97ca5d5b 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -85,7 +85,7 @@ def __init__(self, target, retry, timeout, metadata=None): timeout = TimeToDeadlineTimeout(timeout=timeout) self._timeout = timeout self._metadata = metadata - + def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): """Invoke the low-level RPC with retry, timeout, and metadata.""" From 0831dbfc48b74a1f40e2f6fe6ad9875bf9537892 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Sep 2023 01:12:03 +0000 Subject: [PATCH 08/23] fixed comment typo --- google/api_core/gapic_v1/method.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 97ca5d5b..07c207e8 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -88,8 +88,8 @@ def __init__(self, target, retry, timeout, metadata=None): def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): """Invoke the low-level RPC with retry, timeout, and metadata.""" - - # onstruct wrapped function + + # construct wrapped function wrapped_func = self._target if timeout is DEFAULT: timeout = self._timeout From a1563d20845d2d9387ecdf3a7945d7c74e8c0511 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Sep 2023 01:12:54 +0000 Subject: [PATCH 09/23] fixed spacing --- google/api_core/gapic_v1/method.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 07c207e8..ab257f6b 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -111,7 +111,7 @@ def __call__(self, *args, timeout=DEFAULT, retry=DEFAULT, **kwargs): if metadata is None: metadata = [] kwargs["metadata"] = (*self._metadata, *metadata) - + return wrapped_func(*args, **kwargs) From 85e21029cb9549942380894453de1d1f1cfb5428 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 16:38:45 -0800 Subject: [PATCH 10/23] fixed spacing --- google/api_core/gapic_v1/method.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index 69aee9a8..bd91a9ce 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -115,10 +115,10 @@ def __call__( retry = self._retry if retry is not None: wrapped_func = retry(wrapped_func) - + if compression is DEFAULT: compression = self._compression - if self._compression is not None: + if self._compression is not None: kwargs["compression"] = compression # Add the user agent metadata to the call. From c76f51cd9ffaf99052a0029ece38785369605e54 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 17:02:36 -0800 Subject: [PATCH 11/23] removed unneeded helpers --- google/api_core/gapic_v1/method.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index bd91a9ce..b90387b0 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -42,24 +42,6 @@ class _MethodDefault(enum.Enum): so the default should be used.""" -def _is_not_none_or_false(value): - return value is not None and value is not False - - -def _apply_decorators(func, decorators): - """Apply a list of decorators to a given function. - - ``decorators`` may contain items that are ``None`` or ``False`` which will - be ignored. - """ - filtered_decorators = filter(_is_not_none_or_false, reversed(decorators)) - - for decorator in filtered_decorators: - func = decorator(func) - - return func - - class _GapicCallable(object): """Callable that applies retry, timeout, and metadata logic. From f4a9021472d3d2c3b440a092ab540875eee70677 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 17:03:00 -0800 Subject: [PATCH 12/23] use caching --- google/api_core/gapic_v1/method.py | 37 ++++++++++++++++++------------ 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index b90387b0..f2cf2281 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -84,20 +84,6 @@ def __call__( ): """Invoke the low-level RPC with retry, timeout, compression, and metadata.""" - # construct wrapped function - wrapped_func = self._target - if timeout is DEFAULT: - timeout = self._timeout - elif isinstance(timeout, (int, float)): - timeout = TimeToDeadlineTimeout(timeout=timeout) - if timeout is not None: - wrapped_func = timeout(wrapped_func) - - if retry is DEFAULT: - retry = self._retry - if retry is not None: - wrapped_func = retry(wrapped_func) - if compression is DEFAULT: compression = self._compression if self._compression is not None: @@ -112,7 +98,28 @@ def __call__( metadata = [] kwargs["metadata"] = (*self._metadata, *metadata) - return wrapped_func(*args, **kwargs) + call = self._build_wrapped_call(timeout, retry) + return call(*args, **kwargs) + + @functools.lru_cache + def _build_wrapped_call(self, timeout, retry): + """Invoke the low-level RPC with retry, timeout, compression, and metadata.""" + + # construct wrapped function + wrapped_func = self._target + if timeout is DEFAULT: + timeout = self._timeout + elif isinstance(timeout, (int, float)): + timeout = TimeToDeadlineTimeout(timeout=timeout) + if timeout is not None: + wrapped_func = timeout(wrapped_func) + + if retry is DEFAULT: + retry = self._retry + if retry is not None: + wrapped_func = retry(wrapped_func) + + return wrapped_func def wrap_method( From cacc73c6418659b7f503f50ea25fa6b7298279ad Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 17:12:28 -0800 Subject: [PATCH 13/23] improved metadata parsing --- google/api_core/gapic_v1/method.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index f2cf2281..e5e464fa 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -91,12 +91,12 @@ def __call__( # Add the user agent metadata to the call. if self._metadata is not None: - metadata = kwargs.get("metadata", []) - # Due to the nature of invocation, None should be treated the same - # as not specified. - if metadata is None: - metadata = [] - kwargs["metadata"] = (*self._metadata, *metadata) + try: + # attempt to concatenate default metadata with user-provided metadata + kwargs["metadata"] = (*kwargs["metadata"], *self._metadata) + except KeyError: + # if metadata is not provided, use just the default metadata + kwargs["metadata"] = self._metadata call = self._build_wrapped_call(timeout, retry) return call(*args, **kwargs) From a30101d1574036beb66b818922f8d7745d1d0452 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 17:20:51 -0800 Subject: [PATCH 14/23] improved docstring --- google/api_core/gapic_v1/method.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index e5e464fa..b5cce98a 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -101,11 +101,11 @@ def __call__( call = self._build_wrapped_call(timeout, retry) return call(*args, **kwargs) - @functools.lru_cache + @functools.lru_cache(maxsize=4) def _build_wrapped_call(self, timeout, retry): - """Invoke the low-level RPC with retry, timeout, compression, and metadata.""" - - # construct wrapped function + """ + Build a wrapped callable that applies retry, timeout, and metadata logic. + """ wrapped_func = self._target if timeout is DEFAULT: timeout = self._timeout From db9a9c4edbeec70d3d9c30ba190119aa4a73baf1 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 9 Feb 2024 17:30:13 -0800 Subject: [PATCH 15/23] fixed logic --- google/api_core/gapic_v1/method.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/api_core/gapic_v1/method.py b/google/api_core/gapic_v1/method.py index b5cce98a..206549ea 100644 --- a/google/api_core/gapic_v1/method.py +++ b/google/api_core/gapic_v1/method.py @@ -86,7 +86,7 @@ def __call__( if compression is DEFAULT: compression = self._compression - if self._compression is not None: + if compression is not None: kwargs["compression"] = compression # Add the user agent metadata to the call. @@ -94,7 +94,7 @@ def __call__( try: # attempt to concatenate default metadata with user-provided metadata kwargs["metadata"] = (*kwargs["metadata"], *self._metadata) - except KeyError: + except (KeyError, TypeError): # if metadata is not provided, use just the default metadata kwargs["metadata"] = self._metadata From a5556291056c12c65be2073e9cd7089d5ca1c59b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 13 Feb 2024 14:18:27 -0800 Subject: [PATCH 16/23] added benchmark test --- tests/unit/gapic/test_method.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index d966f478..19fce939 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -222,3 +222,23 @@ def test_wrap_method_with_call_not_supported(): with pytest.raises(ValueError) as exc_info: google.api_core.gapic_v1.method.wrap_method(method, with_call=True) assert "with_call=True is only supported for unary calls" in str(exc_info.value) + + +def test_benchmark_gapic_call(): + """ + Ensure the __call__ method performance does not regress from expectations + + __call__ builds a new wrapped function using passed-in timeout and retry, but + subsequent calls are cached + + https://github.com/googleapis/python-api-core/pull/527 + """ + from google.api_core.gapic_v1.method import _GapicCallable + from google.api_core.retry import Retry + from timeit import timeit + + gapic_callable = _GapicCallable( + lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False + ) + avg_time = timeit(lambda: gapic_callable(), number=10_000) + assert avg_time < 0.15 # expect ~0.1, but allow for some variance From fbbaaca6310189c66ec2682b77b84de03684a8e7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 14 Feb 2024 16:10:58 -0800 Subject: [PATCH 17/23] update threshold --- tests/unit/gapic/test_method.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 19fce939..2c78d912 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -231,6 +231,9 @@ def test_benchmark_gapic_call(): __call__ builds a new wrapped function using passed-in timeout and retry, but subsequent calls are cached + Note: The threshold has been tuned for the CI workers. Test may flake on + slower hardware + https://github.com/googleapis/python-api-core/pull/527 """ from google.api_core.gapic_v1.method import _GapicCallable @@ -241,4 +244,4 @@ def test_benchmark_gapic_call(): lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False ) avg_time = timeit(lambda: gapic_callable(), number=10_000) - assert avg_time < 0.15 # expect ~0.1, but allow for some variance + assert avg_time < 0.4 # expect ~0.15, but allow for some variance From 576bb0f167a724ac68b94e2148dae65bd424c1f6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 16 Feb 2024 12:27:04 -0800 Subject: [PATCH 18/23] run benchmark in loop for testing --- tests/unit/gapic/test_method.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 2c78d912..2f015cdf 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -243,5 +243,7 @@ def test_benchmark_gapic_call(): gapic_callable = _GapicCallable( lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False ) - avg_time = timeit(lambda: gapic_callable(), number=10_000) - assert avg_time < 0.4 # expect ~0.15, but allow for some variance + for i in range(1000): + avg_time = timeit(lambda: gapic_callable(), number=10_000) + print(f"avg_time: {avg_time}") + assert avg_time < 0.4 # expect ~0.15, but allow for some variance From 7c32a5d7f882755c02094ac20d612150fa11495b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 16 Feb 2024 12:52:50 -0800 Subject: [PATCH 19/23] use verbose logs --- .github/workflows/unittest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unittest.yml b/.github/workflows/unittest.yml index 2cfaada3..eae04e0f 100644 --- a/.github/workflows/unittest.yml +++ b/.github/workflows/unittest.yml @@ -41,7 +41,7 @@ jobs: env: COVERAGE_FILE: .coverage${{ matrix.option }}-${{matrix.python }} run: | - nox -s unit${{ matrix.option }}-${{ matrix.python }} + nox -s unit${{ matrix.option }}-${{ matrix.python }} -- -s -v - name: Upload coverage results uses: actions/upload-artifact@v3 with: From 26bec799f378b82790e3cc440e00887ad6e81889 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 16 Feb 2024 12:58:41 -0800 Subject: [PATCH 20/23] Revert testing --- .github/workflows/unittest.yml | 2 +- tests/unit/gapic/test_method.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/unittest.yml b/.github/workflows/unittest.yml index eae04e0f..2cfaada3 100644 --- a/.github/workflows/unittest.yml +++ b/.github/workflows/unittest.yml @@ -41,7 +41,7 @@ jobs: env: COVERAGE_FILE: .coverage${{ matrix.option }}-${{matrix.python }} run: | - nox -s unit${{ matrix.option }}-${{ matrix.python }} -- -s -v + nox -s unit${{ matrix.option }}-${{ matrix.python }} - name: Upload coverage results uses: actions/upload-artifact@v3 with: diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 2f015cdf..2c78d912 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -243,7 +243,5 @@ def test_benchmark_gapic_call(): gapic_callable = _GapicCallable( lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False ) - for i in range(1000): - avg_time = timeit(lambda: gapic_callable(), number=10_000) - print(f"avg_time: {avg_time}") - assert avg_time < 0.4 # expect ~0.15, but allow for some variance + avg_time = timeit(lambda: gapic_callable(), number=10_000) + assert avg_time < 0.4 # expect ~0.15, but allow for some variance From c25e0eb8fe42df73b9412853b9969e30ea036c81 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 16 Feb 2024 13:02:20 -0800 Subject: [PATCH 21/23] used smaller value --- tests/unit/gapic/test_method.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 2c78d912..427bdf33 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -244,4 +244,4 @@ def test_benchmark_gapic_call(): lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False ) avg_time = timeit(lambda: gapic_callable(), number=10_000) - assert avg_time < 0.4 # expect ~0.15, but allow for some variance + assert avg_time < 0.15 # expect <0.06, but allow for some variance From 49201ca7d223bae8d0f9fd39a9486f8dbf80b899 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 16 Feb 2024 13:32:16 -0800 Subject: [PATCH 22/23] changed threshold --- tests/unit/gapic/test_method.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 427bdf33..08320d7c 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -244,4 +244,4 @@ def test_benchmark_gapic_call(): lambda *a, **k: 1, retry=Retry(), timeout=1010, compression=False ) avg_time = timeit(lambda: gapic_callable(), number=10_000) - assert avg_time < 0.15 # expect <0.06, but allow for some variance + assert avg_time < 0.4 From deca58ce5993e123c4c7c82e13100aa6b71f8a7b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 3 May 2024 15:00:35 -0500 Subject: [PATCH 23/23] removed link in comment --- tests/unit/gapic/test_method.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/gapic/test_method.py b/tests/unit/gapic/test_method.py index 08320d7c..370da501 100644 --- a/tests/unit/gapic/test_method.py +++ b/tests/unit/gapic/test_method.py @@ -233,8 +233,6 @@ def test_benchmark_gapic_call(): Note: The threshold has been tuned for the CI workers. Test may flake on slower hardware - - https://github.com/googleapis/python-api-core/pull/527 """ from google.api_core.gapic_v1.method import _GapicCallable from google.api_core.retry import Retry