From bccbda7f28f02109288aeb86e816fb1dce56db3d Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 1 Jun 2020 11:34:35 -0700 Subject: [PATCH 01/17] Add failing test --- .../grpcio_tests/tests/unit/BUILD.bazel | 1 + .../unit/_contextvars_propagation_test.py | 112 ++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py diff --git a/src/python/grpcio_tests/tests/unit/BUILD.bazel b/src/python/grpcio_tests/tests/unit/BUILD.bazel index 42b99023463..690397942cc 100644 --- a/src/python/grpcio_tests/tests/unit/BUILD.bazel +++ b/src/python/grpcio_tests/tests/unit/BUILD.bazel @@ -13,6 +13,7 @@ GRPCIO_TESTS_UNIT = [ "_channel_connectivity_test.py", "_channel_ready_future_test.py", "_compression_test.py", + "_contextvars_propagation_test.py", "_credentials_test.py", "_dns_resolver_test.py", "_empty_message_test.py", diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py new file mode 100644 index 00000000000..dc4a7e7551f --- /dev/null +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -0,0 +1,112 @@ +# Copyright 2020 The gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Test of propagation of contextvars between threads.""" + +import contextlib +import logging +import sys +import unittest + +import grpc + +from tests.unit import test_common + +_UNARY_UNARY = "/test/UnaryUnary" +_REQUEST = b"0000" + + +def _unary_unary_handler(request, context): + return request + + +# TODO: Test for <3.7 and 3.7+. + + +def contextvars_supported(): + return sys.version_info[0] == 3 and sys.version_info[1] >= 7 + + +class _GenericHandler(grpc.GenericRpcHandler): + + def service(self, handler_call_details): + if handler_call_details.method == _UNARY_UNARY: + return grpc.unary_unary_rpc_method_handler(_unary_unary_handler) + else: + raise NotImplementedError() + + +@contextlib.contextmanager +def _server(): + try: + server = test_common.test_server() + target = '[::]:0' + port = server.add_insecure_port(target) + server.add_generic_rpc_handlers((_GenericHandler(),)) + server.start() + yield port + finally: + server.stop(None) + + +if contextvars_supported(): + import contextvars + + _EXPECTED_VALUE = 24601 + test_var = contextvars.ContextVar("test_var", default=None) + test_var.set(_EXPECTED_VALUE) + + class TestCallCredentials(grpc.AuthMetadataPlugin): + + def __init__(self): + self._recorded_value = None + + def __call__(self, context, callback): + self._recorded_value = test_var.get() + callback((), None) + + def assert_called(self, test): + test.assertEqual(_EXPECTED_VALUE, self._recorded_value) + +else: + + class TestCallCredentials(grpc.AuthMetadataPlugin): + + def __call__(self, context, callback): + callback((), None) + + def assert_called(self, test): + pass + + +class ContextVarsPropagationTest(unittest.TestCase): + + def test_propagation_to_auth_plugin(self): + with _server() as port: + target = "localhost:{}".format(port) + local_credentials = grpc.local_channel_credentials() + test_call_credentials = TestCallCredentials() + call_credentials = grpc.metadata_call_credentials( + test_call_credentials, "test call credentials") + composite_credentials = grpc.composite_channel_credentials( + local_credentials, call_credentials) + with grpc.secure_channel(target, composite_credentials) as channel: + stub = channel.unary_unary(_UNARY_UNARY) + response = stub(_REQUEST) + self.assertEqual(_REQUEST, response) + test_call_credentials.assert_called(self) + + +if __name__ == '__main__': + logging.basicConfig() + unittest.main(verbosity=2) From 8487ce7fafd60b9964f95ce282c84ea6dd893c55 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 1 Jun 2020 12:05:15 -0700 Subject: [PATCH 02/17] Propagate on posix --- .../grpc/_cython/_cygrpc/fork_posix.pyx.pxi | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi index 55c8673dd4d..9de22ae6c5a 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi @@ -94,6 +94,26 @@ def fork_handlers_and_grpc_init(): _fork_state.fork_handler_registered = True +def _contextvars_supported(): + return sys.version_info[0] == 3 and sys.version_info[1] >= 7 + + +if _contextvars_supported(): + import contextvars + def _run_with_context(target): + ctx = contextvars.copy_context() + def _run(*args): + ctx.run(target, *args) + return _run +else: + # NOTE(rbellevi): `contextvars` was not introduced until 3.7. On earlier + # interpreters, we simply do not propagate contextvars between threads. + def _run_with_context(target): + def _run(*args): + target(*args) + return _run + + class ForkManagedThread(object): def __init__(self, target, args=()): if _GRPC_ENABLE_FORK_SUPPORT: @@ -102,9 +122,9 @@ class ForkManagedThread(object): target(*args) finally: _fork_state.active_thread_count.decrement() - self._thread = threading.Thread(target=managed_target, args=args) + self._thread = threading.Thread(target=_run_with_context(managed_target), args=args) else: - self._thread = threading.Thread(target=target, args=args) + self._thread = threading.Thread(target=_run_with_context(target), args=args) def setDaemon(self, daemonic): self._thread.daemon = daemonic From 3ade93b447bd4af44909fe74652380bafcd175e7 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 1 Jun 2020 12:19:06 -0700 Subject: [PATCH 03/17] Add TODOs --- src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi | 1 + .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi index 9167cb45173..f0faa0907f1 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi @@ -19,6 +19,7 @@ def fork_handlers_and_grpc_init(): grpc_init() +# TODO: Propagate contextvars. class ForkManagedThread(object): def __init__(self, target, args=()): self._thread = threading.Thread(target=target, args=args) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index dc4a7e7551f..fe675ec7550 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -106,6 +106,8 @@ class ContextVarsPropagationTest(unittest.TestCase): self.assertEqual(_REQUEST, response) test_call_credentials.assert_called(self) + # TODO: Test simple unary-unary. + if __name__ == '__main__': logging.basicConfig() From 2d4b6e894b33e57e79fe7cdba8306ccbe3eb154a Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 1 Jun 2020 15:24:06 -0700 Subject: [PATCH 04/17] Interoperate with contextvars backports --- .../tests/unit/_contextvars_propagation_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index fe675ec7550..3a739f05154 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -34,7 +34,11 @@ def _unary_unary_handler(request, context): def contextvars_supported(): - return sys.version_info[0] == 3 and sys.version_info[1] >= 7 + try: + import contextvars + return True + except ImportError: + return False class _GenericHandler(grpc.GenericRpcHandler): From c843c1880153a35079e159fbbe1085fc9aaf0c11 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Mon, 1 Jun 2020 15:28:25 -0700 Subject: [PATCH 05/17] Actually change the implementation --- src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi index 9de22ae6c5a..7133d52c3e4 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi @@ -94,8 +94,12 @@ def fork_handlers_and_grpc_init(): _fork_state.fork_handler_registered = True -def _contextvars_supported(): - return sys.version_info[0] == 3 and sys.version_info[1] >= 7 +def contextvars_supported(): + try: + import contextvars + return True + except ImportError: + return False if _contextvars_supported(): From f4d420012794aa1e3cc16e804ea257360ae2ff9c Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 2 Jun 2020 10:39:40 -0700 Subject: [PATCH 06/17] Underscore --- src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi index 7133d52c3e4..437ee8123b5 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi @@ -94,7 +94,7 @@ def fork_handlers_and_grpc_init(): _fork_state.fork_handler_registered = True -def contextvars_supported(): +def _contextvars_supported(): try: import contextvars return True From 2ed3d6bb5ea041273ff34ea74052357727e7b638 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 2 Jun 2020 11:20:14 -0700 Subject: [PATCH 07/17] Add to tests.json --- src/python/grpcio_tests/tests/tests.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/grpcio_tests/tests/tests.json b/src/python/grpcio_tests/tests/tests.json index 196e9f08b0a..c7d913f49ca 100644 --- a/src/python/grpcio_tests/tests/tests.json +++ b/src/python/grpcio_tests/tests/tests.json @@ -35,6 +35,7 @@ "unit._channel_connectivity_test.ChannelConnectivityTest", "unit._channel_ready_future_test.ChannelReadyFutureTest", "unit._compression_test.CompressionTest", + "unit._contextvars_propagation_test.ContextVarsPropagationTest", "unit._credentials_test.CredentialsTest", "unit._cython._cancel_many_calls_test.CancelManyCallsTest", "unit._cython._channel_test.ChannelTest", From bc157d5eb5b85f467d91ca3d7969b5bf262e3fe1 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 2 Jun 2020 11:46:11 -0700 Subject: [PATCH 08/17] Support Windows --- .../grpc/_cython/_cygrpc/fork_posix.pyx.pxi | 22 ------- .../grpc/_cython/_cygrpc/fork_windows.pyx.pxi | 3 +- .../grpc/_cython/_cygrpc/thread.pyx.pxi | 60 +++++++++++++++++++ src/python/grpcio/grpc/_cython/cygrpc.pyx | 2 + .../unit/_contextvars_propagation_test.py | 7 +-- 5 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi index 437ee8123b5..53657e8b1a9 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_posix.pyx.pxi @@ -94,28 +94,6 @@ def fork_handlers_and_grpc_init(): _fork_state.fork_handler_registered = True -def _contextvars_supported(): - try: - import contextvars - return True - except ImportError: - return False - - -if _contextvars_supported(): - import contextvars - def _run_with_context(target): - ctx = contextvars.copy_context() - def _run(*args): - ctx.run(target, *args) - return _run -else: - # NOTE(rbellevi): `contextvars` was not introduced until 3.7. On earlier - # interpreters, we simply do not propagate contextvars between threads. - def _run_with_context(target): - def _run(*args): - target(*args) - return _run class ForkManagedThread(object): diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi index f0faa0907f1..f9d41958dc7 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi @@ -19,10 +19,9 @@ def fork_handlers_and_grpc_init(): grpc_init() -# TODO: Propagate contextvars. class ForkManagedThread(object): def __init__(self, target, args=()): - self._thread = threading.Thread(target=target, args=args) + self._thread = threading.Thread(target=_tun_with_context(target), args=args) def setDaemon(self, daemonic): self._thread.daemon = daemonic diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi new file mode 100644 index 00000000000..7ba8cc9af79 --- /dev/null +++ b/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi @@ -0,0 +1,60 @@ +# Copyright 2020 The gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +def _contextvars_supported(): + """Determines if the contextvars module is supported. + + We use a 'try it and see if it works approach' here rather than predicting + based on interpreter version in order to support older interpreters that + may have a backported module based on, e.g. `threading.local`. + + Returns: + A bool indicating whether `contextvars` are supported in the current + environment. + """ + try: + import contextvars + return True + except ImportError: + return False + + +def _run_with_context(target): + """Runs a callable with contextvars propagated. + + If contextvars are supported, the calling thread's context will be copied + and propagated. If they are not supported, this function is equivalent + to the identity function. + + Args: + target: A callable object to wrap. + Returns: + A callable object with the same signature as `target` but with + contextvars propagated. + """ + pass + + +if _contextvars_supported(): + import contextvars + def _run_with_context(target): + ctx = contextvars.copy_context() + def _run(*args): + ctx.run(target, *args) + return _run +else: + def _run_with_context(target): + def _run(*args): + target(*args) + return _run diff --git a/src/python/grpcio/grpc/_cython/cygrpc.pyx b/src/python/grpcio/grpc/_cython/cygrpc.pyx index b0a753c7ebe..0ce8bda0d89 100644 --- a/src/python/grpcio/grpc/_cython/cygrpc.pyx +++ b/src/python/grpcio/grpc/_cython/cygrpc.pyx @@ -59,6 +59,8 @@ include "_cygrpc/iomgr.pyx.pxi" include "_cygrpc/grpc_gevent.pyx.pxi" +include "_cygrpc/thread.pyx.pxi" + IF UNAME_SYSNAME == "Windows": include "_cygrpc/fork_windows.pyx.pxi" ELSE: diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 3a739f05154..9ff5473f165 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -"""Test of propagation of contextvars between threads.""" +"""Test of propagation of contextvars to AuthMetadataPlugin threads..""" import contextlib import logging @@ -30,9 +30,6 @@ def _unary_unary_handler(request, context): return request -# TODO: Test for <3.7 and 3.7+. - - def contextvars_supported(): try: import contextvars @@ -110,8 +107,6 @@ class ContextVarsPropagationTest(unittest.TestCase): self.assertEqual(_REQUEST, response) test_call_credentials.assert_called(self) - # TODO: Test simple unary-unary. - if __name__ == '__main__': logging.basicConfig() From 9e5a91b3b95c4e00660618b0d17ba1708c761839 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Tue, 2 Jun 2020 12:04:11 -0700 Subject: [PATCH 09/17] A rare moment where I wish I had a Windows machine --- src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi index f9d41958dc7..67aaf4d033d 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/fork_windows.pyx.pxi @@ -21,7 +21,7 @@ def fork_handlers_and_grpc_init(): class ForkManagedThread(object): def __init__(self, target, args=()): - self._thread = threading.Thread(target=_tun_with_context(target), args=args) + self._thread = threading.Thread(target=_run_with_context(target), args=args) def setDaemon(self, daemonic): self._thread.daemon = daemonic From fb54a30483429089e4c763475687ade6490bb7cb Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 3 Jun 2020 15:19:37 -0700 Subject: [PATCH 10/17] Ban gevent tests --- src/python/grpcio_tests/commands.py | 3 +++ .../tests/unit/_contextvars_propagation_test.py | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index f7cd7c6b8a1..f6b5859ced4 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -220,6 +220,9 @@ class TestGevent(setuptools.Command): 'unit._cython._channel_test.ChannelTest.test_negative_deadline_connectivity', # TODO(https://github.com/grpc/grpc/issues/15411) enable this test 'unit._local_credentials_test.LocalCredentialsTest', + # TODO(https://github.com/grpc/grpc/issues/22020) LocalCredentials + # aren't supported with custom io managers. + 'unit._contextvars_propagation_test', 'testing._time_test.StrictRealTimeTest', ) BANNED_WINDOWS_TESTS = ( diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 9ff5473f165..1fee901c8f3 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -51,7 +51,7 @@ class _GenericHandler(grpc.GenericRpcHandler): def _server(): try: server = test_common.test_server() - target = '[::]:0' + target = 'localhost:0' port = server.add_insecure_port(target) server.add_generic_rpc_handlers((_GenericHandler(),)) server.start() @@ -65,21 +65,28 @@ if contextvars_supported(): _EXPECTED_VALUE = 24601 test_var = contextvars.ContextVar("test_var", default=None) - test_var.set(_EXPECTED_VALUE) + + def set_up_expected_context(): + test_var.set(_EXPECTED_VALUE) class TestCallCredentials(grpc.AuthMetadataPlugin): def __init__(self): self._recorded_value = None + self._invoked = False def __call__(self, context, callback): self._recorded_value = test_var.get() + self._invoked = True callback((), None) def assert_called(self, test): + test.assertTrue(self._invoked) test.assertEqual(_EXPECTED_VALUE, self._recorded_value) else: + def set_up_expected_context(): + pass class TestCallCredentials(grpc.AuthMetadataPlugin): @@ -93,6 +100,7 @@ else: class ContextVarsPropagationTest(unittest.TestCase): def test_propagation_to_auth_plugin(self): + set_up_expected_context() with _server() as port: target = "localhost:{}".format(port) local_credentials = grpc.local_channel_credentials() From a64dac7b93fd0a8d2e50107eb6b51b7e335ce995 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 3 Jun 2020 15:30:10 -0700 Subject: [PATCH 11/17] Yapf --- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 1fee901c8f3..4459225c958 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -85,6 +85,7 @@ if contextvars_supported(): test.assertEqual(_EXPECTED_VALUE, self._recorded_value) else: + def set_up_expected_context(): pass From 65ef92861df9aede79a853be24c466dc94ccc40d Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Wed, 3 Jun 2020 16:41:44 -0700 Subject: [PATCH 12/17] Re-add wait_for_ready --- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 4459225c958..4daad0b4562 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -112,7 +112,7 @@ class ContextVarsPropagationTest(unittest.TestCase): local_credentials, call_credentials) with grpc.secure_channel(target, composite_credentials) as channel: stub = channel.unary_unary(_UNARY_UNARY) - response = stub(_REQUEST) + response = stub(_REQUEST, wait_for_ready=True) self.assertEqual(_REQUEST, response) test_call_credentials.assert_called(self) From 9fac7a1ad43474949be1e506dcd1b22ac6245223 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Thu, 4 Jun 2020 10:38:09 -0700 Subject: [PATCH 13/17] Work around Windows bug --- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 4daad0b4562..9e3e4beaf68 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -103,7 +103,10 @@ class ContextVarsPropagationTest(unittest.TestCase): def test_propagation_to_auth_plugin(self): set_up_expected_context() with _server() as port: - target = "localhost:{}".format(port) + # NOTE(rbellevi): We use a literal IPV6 address because 'localhost' + # is not recognized as a local address by the LocalCredentials + # implementation on Windows. + target = "[::]:{}".format(port) local_credentials = grpc.local_channel_credentials() test_call_credentials = TestCallCredentials() call_credentials = grpc.metadata_call_credentials( From 773611c1d1d33ce81f9a8c658eea0e85f5c12b3b Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Thu, 4 Jun 2020 12:35:34 -0700 Subject: [PATCH 14/17] Plz Windoze --- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 9e3e4beaf68..538199b6def 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -106,7 +106,7 @@ class ContextVarsPropagationTest(unittest.TestCase): # NOTE(rbellevi): We use a literal IPV6 address because 'localhost' # is not recognized as a local address by the LocalCredentials # implementation on Windows. - target = "[::]:{}".format(port) + target = "[::1]:{}".format(port) local_credentials = grpc.local_channel_credentials() test_call_credentials = TestCallCredentials() call_credentials = grpc.metadata_call_credentials( From bd7291b020ea23aa44d11243863d5daafd63a113 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Thu, 4 Jun 2020 13:56:34 -0700 Subject: [PATCH 15/17] :tableflip: --- src/python/grpcio_tests/commands.py | 5 ++++- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 5 +---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index f6b5859ced4..87724dab774 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -227,7 +227,10 @@ class TestGevent(setuptools.Command): ) BANNED_WINDOWS_TESTS = ( # TODO(https://github.com/grpc/grpc/pull/15411) enable this test - 'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback',) + 'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback', + # TODO(https://github.com/grpc/grpc/issues/22257) + 'unit._contextvars_propagation_test.ContextVarsPropagationTest', + ) description = 'run tests with gevent. Assumes grpc/gevent are installed' user_options = [] diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 538199b6def..4daad0b4562 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -103,10 +103,7 @@ class ContextVarsPropagationTest(unittest.TestCase): def test_propagation_to_auth_plugin(self): set_up_expected_context() with _server() as port: - # NOTE(rbellevi): We use a literal IPV6 address because 'localhost' - # is not recognized as a local address by the LocalCredentials - # implementation on Windows. - target = "[::1]:{}".format(port) + target = "localhost:{}".format(port) local_credentials = grpc.local_channel_credentials() test_call_credentials = TestCallCredentials() call_credentials = grpc.metadata_call_credentials( From 420584462feae63e76d3f04dd7b052694f63b8ba Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Thu, 4 Jun 2020 15:15:05 -0700 Subject: [PATCH 16/17] :chairflip: --- src/python/grpcio_tests/commands.py | 5 +---- .../grpcio_tests/tests/unit/_contextvars_propagation_test.py | 3 +++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 87724dab774..f6b5859ced4 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -227,10 +227,7 @@ class TestGevent(setuptools.Command): ) BANNED_WINDOWS_TESTS = ( # TODO(https://github.com/grpc/grpc/pull/15411) enable this test - 'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback', - # TODO(https://github.com/grpc/grpc/issues/22257) - 'unit._contextvars_propagation_test.ContextVarsPropagationTest', - ) + 'unit._dns_resolver_test.DNSResolverTest.test_connect_loopback',) description = 'run tests with gevent. Assumes grpc/gevent are installed' user_options = [] diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index 4daad0b4562..b892472a2e7 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -15,6 +15,7 @@ import contextlib import logging +import os import sys import unittest @@ -98,6 +99,8 @@ else: pass +# TODO(https://github.com/grpc/grpc/issues/22257) +@unittest.skipIf(os.name == "nt", "LocalCredentials not supported on Windows.") class ContextVarsPropagationTest(unittest.TestCase): def test_propagation_to_auth_plugin(self): From a0e23e21234af542c5786b864ec380536ae411be Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 5 Jun 2020 12:30:09 -0700 Subject: [PATCH 17/17] Review comments --- .../grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi | 1 - .../tests/unit/_contextvars_propagation_test.py | 13 +++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi index 7ba8cc9af79..be4cb8b9a8e 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/thread.pyx.pxi @@ -43,7 +43,6 @@ def _run_with_context(target): A callable object with the same signature as `target` but with contextvars propagated. """ - pass if _contextvars_supported(): diff --git a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py index b892472a2e7..fec0fbd7df4 100644 --- a/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py +++ b/src/python/grpcio_tests/tests/unit/_contextvars_propagation_test.py @@ -72,13 +72,10 @@ if contextvars_supported(): class TestCallCredentials(grpc.AuthMetadataPlugin): - def __init__(self): - self._recorded_value = None - self._invoked = False - def __call__(self, context, callback): - self._recorded_value = test_var.get() - self._invoked = True + if test_var.get() != _EXPECTED_VALUE: + raise AssertionError("{} != {}".format(test_var.get(), + _EXPECTED_VALUE)) callback((), None) def assert_called(self, test): @@ -95,9 +92,6 @@ else: def __call__(self, context, callback): callback((), None) - def assert_called(self, test): - pass - # TODO(https://github.com/grpc/grpc/issues/22257) @unittest.skipIf(os.name == "nt", "LocalCredentials not supported on Windows.") @@ -117,7 +111,6 @@ class ContextVarsPropagationTest(unittest.TestCase): stub = channel.unary_unary(_UNARY_UNARY) response = stub(_REQUEST, wait_for_ready=True) self.assertEqual(_REQUEST, response) - test_call_credentials.assert_called(self) if __name__ == '__main__':