From 363782b27369bd565ff1b51a1622774e1e530e7c Mon Sep 17 00:00:00 2001 From: Felipe Selmo Date: Tue, 1 Feb 2022 17:33:35 -0700 Subject: [PATCH 1/4] Properly initialize external modules Properly initialize modules that do not inherit from the `web3.module.Module` class. We weren't properly testing self-referential, non-static methods with this functionality and so a test was also added for this. --- newsfragments/2328.bugfix.rst | 1 + tests/core/conftest.py | 16 +++++++++++----- tests/core/utilities/test_attach_modules.py | 9 +++++---- tests/core/web3-module/test_attach_modules.py | 9 +++++---- web3/_utils/module.py | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 newsfragments/2328.bugfix.rst diff --git a/newsfragments/2328.bugfix.rst b/newsfragments/2328.bugfix.rst new file mode 100644 index 0000000000..ef7bbad1d1 --- /dev/null +++ b/newsfragments/2328.bugfix.rst @@ -0,0 +1 @@ +Properly initialize external modules that do not inherit from the ``web3.module.Module`` class \ No newline at end of file diff --git a/tests/core/conftest.py b/tests/core/conftest.py index cdaa362fce..cdc01b255a 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -50,29 +50,35 @@ class Module4(Module): def module1_unique(): class Module1: a = 'a' + + def __init__(self): + self._b = "b" + + def b(self): + return self._b return Module1 @pytest.fixture(scope='module') def module2_unique(): class Module2: - b = 'b' + c = 'c' @staticmethod - def c(): - return 'c' + def d(): + return 'd' return Module2 @pytest.fixture(scope='module') def module3_unique(): class Module3: - d = 'd' + e = 'e' return Module3 @pytest.fixture(scope='module') def module4_unique(): class Module4: - e = 'e' + f = 'f' return Module4 diff --git a/tests/core/utilities/test_attach_modules.py b/tests/core/utilities/test_attach_modules.py index 60914dc21f..db7c01728f 100644 --- a/tests/core/utilities/test_attach_modules.py +++ b/tests/core/utilities/test_attach_modules.py @@ -147,16 +147,17 @@ def test_attach_external_modules_that_do_not_inherit_from_module_class( # assert module1 attached assert hasattr(w3, 'module1') assert w3.module1.a == 'a' + assert w3.module1.b() == 'b' # assert module2 + submodules attached assert hasattr(w3, 'module2') - assert w3.module2.b == 'b' - assert w3.module2.c() == 'c' + assert w3.module2.c == 'c' + assert w3.module2.d() == 'd' assert hasattr(w3.module2, 'submodule1') - assert w3.module2.submodule1.d == 'd' + assert w3.module2.submodule1.e == 'e' assert hasattr(w3.module2.submodule1, 'submodule2') - assert w3.module2.submodule1.submodule2.e == 'e' + assert w3.module2.submodule1.submodule2.f == 'f' # assert default modules intact assert hasattr(w3, 'geth') diff --git a/tests/core/web3-module/test_attach_modules.py b/tests/core/web3-module/test_attach_modules.py index 58b3d5aca3..a5e7f3f44b 100644 --- a/tests/core/web3-module/test_attach_modules.py +++ b/tests/core/web3-module/test_attach_modules.py @@ -51,16 +51,17 @@ def test_attach_modules_that_do_not_inherit_from_module_class( # assert module1 attached assert hasattr(web3, 'module1') assert web3.module1.a == 'a' + assert web3.module1.b() == 'b' # assert module2 + submodules attached assert hasattr(web3, 'module2') - assert web3.module2.b == 'b' - assert web3.module2.c() == 'c' + assert web3.module2.c == 'c' + assert web3.module2.d() == 'd' assert hasattr(web3.module2, 'submodule1') - assert web3.module2.submodule1.d == 'd' + assert web3.module2.submodule1.e == 'e' assert hasattr(web3.module2.submodule1, 'submodule2') - assert web3.module2.submodule1.submodule2.e == 'e' + assert web3.module2.submodule1.submodule2.f == 'f' # assert default modules intact assert hasattr(web3, 'geth') diff --git a/web3/_utils/module.py b/web3/_utils/module.py index 89142583de..0b76882bd0 100644 --- a/web3/_utils/module.py +++ b/web3/_utils/module.py @@ -45,7 +45,7 @@ def attach_modules( setattr(parent_module, module_name, module_class(w3)) else: # An external `module_class` need not inherit from the `web3.module.Module` class. - setattr(parent_module, module_name, module_class) + setattr(parent_module, module_name, module_class()) if module_info_is_list_like: if len(module_info) == 2: From 210316a3ecf4a3684d434074e1771730ba037cb9 Mon Sep 17 00:00:00 2001 From: Marc Garreau Date: Wed, 2 Feb 2022 11:54:41 -0700 Subject: [PATCH 2/4] correct docs for external modules --- docs/web3.main.rst | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/docs/web3.main.rst b/docs/web3.main.rst index 3feeefb684..d50c0b7b92 100644 --- a/docs/web3.main.rst +++ b/docs/web3.main.rst @@ -419,17 +419,15 @@ web3.py library. External Modules ~~~~~~~~~~~~~~~~ -External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. Adding external modules -can occur either at instantiation of the ``Web3`` instance or by making use of the ``attach_modules()`` method. - -Unlike the native modules, external modules need not inherit from the ``web3.module.Module`` class. The only requirement -is that a Module must be a class and, if you'd like to make use of the parent ``Web3`` instance, it must be passed into -the ``__init__`` function. For example: +External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. They are not required to +utilize the parent ``Web3`` instance; if no reference is required, the external module need only be a standard class. +If, however, you do want to reference the parent ``Web3`` object, the external module must inherit from the +``web3.module.Module`` class and handle the instance as an argument within the ``__init__`` function: .. code-block:: python - >>> class ExampleModule(): - ... + >>> from web3.module import Module + >>> class ExampleModule(Module): ... def __init__(self, w3): ... self.w3 = w3 ... @@ -440,7 +438,8 @@ the ``__init__`` function. For example: .. warning:: Given the flexibility of external modules, use caution and only import modules from trusted third parties and open source code you've vetted! -To instantiate the ``Web3`` instance with external modules: +Configuring external modules can occur either at instantiation of the ``Web3`` instance or by making use of the +``attach_modules()`` method. To instantiate the ``Web3`` instance with external modules: .. code-block:: python @@ -466,11 +465,11 @@ To instantiate the ``Web3`` instance with external modules: ... ) # `return_zero`, in this case, is an example attribute of the `ModuleClass1` object - >>> w3.module1.return_zero + >>> w3.module1.return_zero() 0 - >>> w3.module2.submodule1.return_one + >>> w3.module2.submodule1.return_one() 1 - >>> w3.module2.submodule2.submodule2a.return_two + >>> w3.module2.submodule2.submodule2a.return_two() 2 @@ -504,9 +503,9 @@ To instantiate the ``Web3`` instance with external modules: ... }) ... }) ... }) - >>> w3.module1.return_zero + >>> w3.module1.return_zero() 0 - >>> w3.module2.submodule1.return_one + >>> w3.module2.submodule1.return_one() 1 - >>> w3.module2.submodule2.submodule2a.return_two + >>> w3.module2.submodule2.submodule2a.return_two() 2 From 67f0562584efb4ae3cc399f68c7f694195318339 Mon Sep 17 00:00:00 2001 From: Felipe Selmo Date: Wed, 2 Feb 2022 16:37:53 -0700 Subject: [PATCH 3/4] Refactor attach_module logic * Allow for accepting the ``Web3`` instance as the first argument in any module's ``__init()`` method, rather than requiring a module to inherit from ``web3.module.Module`` if it needs to make use of the ``Web3`` instance. * Update tests to test the above change. * Add a more friendly error message if the module has more than one __init__() argument. Add test for this error message / case. --- tests/core/conftest.py | 17 +++++++- tests/core/utilities/test_attach_modules.py | 18 ++++++++ tests/core/web3-module/test_attach_modules.py | 17 ++++++++ web3/_utils/module.py | 43 ++++++++++++++----- 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index cdc01b255a..6947f8d334 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -48,14 +48,20 @@ class Module4(Module): @pytest.fixture(scope='module') def module1_unique(): + # uses ``Web3`` instance by accepting it as first arg in the ``__init__()`` method class Module1: a = 'a' - def __init__(self): + def __init__(self, w3): self._b = "b" + self.w3 = w3 def b(self): return self._b + + @property + def return_eth_chain_id(self): + return self.w3.eth.chain_id return Module1 @@ -82,3 +88,12 @@ def module4_unique(): class Module4: f = 'f' return Module4 + + +@pytest.fixture(scope='module') +def module_many_init_args(): + class ModuleManyArgs: + def __init__(self, a, b): + self.a = a + self.b = b + return ModuleManyArgs diff --git a/tests/core/utilities/test_attach_modules.py b/tests/core/utilities/test_attach_modules.py index db7c01728f..4d151cc73a 100644 --- a/tests/core/utilities/test_attach_modules.py +++ b/tests/core/utilities/test_attach_modules.py @@ -1,3 +1,6 @@ +from io import ( + UnsupportedOperation, +) import pytest from eth_utils import ( @@ -148,6 +151,7 @@ def test_attach_external_modules_that_do_not_inherit_from_module_class( assert hasattr(w3, 'module1') assert w3.module1.a == 'a' assert w3.module1.b() == 'b' + assert w3.module1.return_eth_chain_id == w3.eth.chain_id # assert module2 + submodules attached assert hasattr(w3, 'module2') @@ -163,3 +167,17 @@ def test_attach_external_modules_that_do_not_inherit_from_module_class( assert hasattr(w3, 'geth') assert hasattr(w3, 'eth') assert is_integer(w3.eth.chain_id) + + +def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args): + with pytest.raises( + UnsupportedOperation, + match=( + "A module class may accept a single `Web3` instance as the first argument of its " + "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']" + ) + ): + Web3( + EthereumTesterProvider(), + external_modules={'module_should_fail': module_many_init_args} + ) diff --git a/tests/core/web3-module/test_attach_modules.py b/tests/core/web3-module/test_attach_modules.py index a5e7f3f44b..35f575cb06 100644 --- a/tests/core/web3-module/test_attach_modules.py +++ b/tests/core/web3-module/test_attach_modules.py @@ -1,3 +1,8 @@ +from io import ( + UnsupportedOperation, +) +import pytest + from eth_utils import ( is_integer, ) @@ -52,6 +57,7 @@ def test_attach_modules_that_do_not_inherit_from_module_class( assert hasattr(web3, 'module1') assert web3.module1.a == 'a' assert web3.module1.b() == 'b' + assert web3.module1.return_eth_chain_id == web3.eth.chain_id # assert module2 + submodules attached assert hasattr(web3, 'module2') @@ -67,3 +73,14 @@ def test_attach_modules_that_do_not_inherit_from_module_class( assert hasattr(web3, 'geth') assert hasattr(web3, 'eth') assert is_integer(web3.eth.chain_id) + + +def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args): + with pytest.raises( + UnsupportedOperation, + match=( + "A module class may accept a single `Web3` instance as the first argument of its " + "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']" + ) + ): + web3.attach_modules({'module_should_fail': module_many_init_args}) diff --git a/web3/_utils/module.py b/web3/_utils/module.py index 0b76882bd0..7bba1a86db 100644 --- a/web3/_utils/module.py +++ b/web3/_utils/module.py @@ -1,7 +1,12 @@ +import inspect +from io import ( + UnsupportedOperation, +) from typing import ( TYPE_CHECKING, Any, Dict, + List, Optional, Sequence, Union, @@ -18,6 +23,22 @@ from web3 import Web3 # noqa: F401 +def _validate_init_params_and_return_if_found(module_class: Any) -> List[str]: + init_params_raw = list(inspect.signature(module_class.__init__).parameters) + module_init_params = [ + param for param in init_params_raw if param not in ['self', 'args', 'kwargs'] + ] + + if len(module_init_params) > 1: + raise UnsupportedOperation( + "A module class may accept a single `Web3` instance as the first argument of its " + f"__init__() method. More than one argument found for {module_class.__name__}: " + f"{module_init_params}" + ) + + return module_init_params + + def attach_modules( parent_module: Union["Web3", "Module"], module_definitions: Dict[str, Any], @@ -34,17 +55,19 @@ def attach_modules( "already has an attribute with that name" ) - if issubclass(module_class, Module): - # If the `module_class` inherits from the `web3.module.Module` class, it has access to - # caller functions internal to the web3.py library and sets up a proper codec. This - # is likely important for all modules internal to the library. - if w3 is None: - setattr(parent_module, module_name, module_class(parent_module)) - w3 = parent_module - else: - setattr(parent_module, module_name, module_class(w3)) + # The parent module is the ``Web3`` instance on first run of the loop + if type(parent_module).__name__ == 'Web3': + w3 = parent_module + + module_init_params = _validate_init_params_and_return_if_found(module_class) + if len(module_init_params) == 1: + # Modules that need access to the ``Web3`` instance may accept the instance as the first + # arg in their ``__init__()`` method. This is the case for any module that inherits from + # ``web3.module.Module``. + # e.g. def __init__(self, w3): + setattr(parent_module, module_name, module_class(w3)) else: - # An external `module_class` need not inherit from the `web3.module.Module` class. + # Modules need not take in a ``Web3`` instance in their ``__init__()`` if not needed setattr(parent_module, module_name, module_class()) if module_info_is_list_like: From 2f1c2aeea639364e299ecde806ced04d4f854947 Mon Sep 17 00:00:00 2001 From: Marc Garreau Date: Mon, 7 Feb 2022 12:06:07 -0700 Subject: [PATCH 4/4] recorrect docs for external modules --- docs/web3.main.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/web3.main.rst b/docs/web3.main.rst index d50c0b7b92..f85840c62d 100644 --- a/docs/web3.main.rst +++ b/docs/web3.main.rst @@ -419,15 +419,13 @@ web3.py library. External Modules ~~~~~~~~~~~~~~~~ -External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. They are not required to -utilize the parent ``Web3`` instance; if no reference is required, the external module need only be a standard class. -If, however, you do want to reference the parent ``Web3`` object, the external module must inherit from the -``web3.module.Module`` class and handle the instance as an argument within the ``__init__`` function: +External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. External modules are simply +classes whose methods and properties can be made available within the ``Web3`` instance. Optionally, the external module may +make use of the parent ``Web3`` instance by accepting it as the first argument within the ``__init__`` function: .. code-block:: python - >>> from web3.module import Module - >>> class ExampleModule(Module): + >>> class ExampleModule: ... def __init__(self, w3): ... self.w3 = w3 ... @@ -439,7 +437,8 @@ If, however, you do want to reference the parent ``Web3`` object, the external m and open source code you've vetted! Configuring external modules can occur either at instantiation of the ``Web3`` instance or by making use of the -``attach_modules()`` method. To instantiate the ``Web3`` instance with external modules: +``attach_modules()`` method. To instantiate the ``Web3`` instance with external modules use the ``external_modules`` +keyword argument: .. code-block:: python