outline for async web3 api: implement async Version module #1166

dylanjw · 2018-12-15T01:23:26Z

Related to Issue #1161.

Minimal web3 async api. Includes:

AsyncEthereumTesterProvider was added, which has eth-tester related middlewares removed, with a request function that doesnt await.
request_async was updated in RequestManager to be a coroutine. code copy of the blocking request. Im preferring simple over dry for this push.
Added a framework for new web3 module apis which comprises of:
- AsyncMethod and BlockingMethod callable classes (can be configured for property access)
- Rough method configuration format (json for now).
- ~~web3.Module manages the "attaching" of method callables to the module, via a method_config and method_class.~~ EDIT: methods are attached via a descriptor now.
- Demo of the changes using web3.version.Version module.
- Not built-in to the web3 object. e.g.:

from web3 import Web3
from web3.providers.tester.main import AsyncEthereumTesterProvider
from web3.version import AsyncVersion

w3 = Web3(
    AsyncEthereumTesterProvider, 
    middlewares=[], 
    modules={"async_version": AsyncVersion})

Cute Animal Picture

dylanjw · 2018-12-15T01:44:46Z

web3/method.py

+    'name': "signInBlood",
+    'mungers': [],
+    'json_rpc_method': "eth_signTransactionInBlood",
+}


meant to move this and the test below.

For those interested in what 'mungers' are (thanks for the fun word @carver). It is the list of the functions that the python method arguments get piped through, that should output a json_rpc ready param dict. Havent found a name I like better. Because these functions can be validators, normalizers, formatters, occur in any order, with probably a huge variation arity, etc., "munger" seems appropriate. These are being kept separate from the request parameter and result formatters, taken from the middleware formatters.

I'm good with "mungers" unless something better surfaces. Maybe good to try and find a good place to write up a nice comment documenting the validator/normalizer/formatter concepts it encapsulates

dylanjw · 2018-12-15T01:51:30Z

web3/method.py

+    """
+    for fn in fns:
+        val = fn(val)
+        yield val


move to utils

dylanjw · 2018-12-15T01:53:29Z

web3/module.py

+                self.method_class,
+                attr)
+        except UndefinedMethodError:
+            raise AttributeError


Im wondering if it is worth caching the method object after the first lookup.

pipermerriam · 2018-12-17T18:09:25Z

web3/manager.py

+    async def _make_async_request(self, method, params):
+        """TODO: Can this be made dry? Going for dumb solution for now.
+        """
+        for provider in self.providers:


We should be able to drop this loop in v5 by removing the multi-provider API. cc @kclowes

pipermerriam · 2018-12-17T18:10:12Z

web3/manager.py

@@ -102,6 +98,26 @@ def _make_request(self, method, params):
                )
            )

+    async def _make_async_request(self, method, params):


Naming convention for this type of thing has been to use a coro_ prefix so maybe _coro_make_request.

pipermerriam · 2018-12-17T18:34:03Z

web3/method.py

+    'name': "signInBlood",
+    'mungers': [],
+    'json_rpc_method': "eth_signTransactionInBlood",
+}


I'm good with "mungers" unless something better surfaces. Maybe good to try and find a good place to write up a nice comment documenting the validator/normalizer/formatter concepts it encapsulates

pipermerriam · 2018-12-17T18:43:25Z

web3/method.py

+    def __init__(self, web3, method_config):
+        self.__name__ = method_config.get('name', 'anonymous')
+        self.__doc__ = method_config.get('doc', '')
+        self.is_property = method_config.get('is_property', False)


What do you think about removing this and only implementing the concept of direct property lookups at the Module level?

class VersionModule: get_version = VersionMethod(...) @property def version(self): return self.get_version()

Would work similarly for async

pipermerriam · 2018-12-17T18:46:15Z

web3/method.py

+
+
+@to_tuple
+def pipe_appends(fns, val):


Interesting, it's kind of like https://toolz.readthedocs.io/en/latest/api.html#toolz.itertoolz.accumulate combined with pipe. Maybe re-use the name and call this accumulate_pipe or pipe_and_accumulate?

pipermerriam · 2018-12-17T18:59:06Z

web3/module.py

+        if self.module_config is None:
+            self.module_config = dict()
+
+    def __getattr__(self, attr):


I'm 👎 on using __getattr__. It makes introspection and type hinting not work well.

I'd advocate for something like:

class Module: get_version: GetVersionMethod # this is how we get strong-ish type hints. def __init__(self, ...): self.get_version = GetVersion(...)

In theory we can do this by assigning them at the class level and using the descriptor API to get access to the w3 instance.

dylanjw · 2018-12-19T22:50:01Z

I want to restate the goals for this pull request now that it has been worked through further. It is basically restating the opening description, but with more clarity on my part.

This PRs primary intention is to implement the Method objects and the strategy for sync and async execution. More specifically it aims to determine the association between the modules, method definitions and async and sync calling functions.

The following is a description of the structure ive chosen:

The Method class is in charge of interpreting a method configuration. Meaning the Method is concerned with collecting the method lookup, mungers, formatters and executing them on the inputs.

In this first version the method configuration is a dict, ~~but I see these becoming a small class that can validate and document the configuration requirements.~~

The Method class has a secondary concern with directing the calls to the method to a delegated call function. The Method class defines __get__ method, becoming a descriptor. This overrides attribute access from the module with __get__ which is passed the object or the object class. In this way the module is able to define the caller function delegate, rather than in the Method object. This saves having to instantiate the Method objects for both async and sync modules.

The method does not retain any state from the module object. The __get__ method just passes itself (the Method object) and the module web3 instance to the call function delegate. The method object provides the input processing and the web3 instance provides the manager request function.

Using this structure the Method objects can be defined once in a shared base class. To make this safe, I need to find a way to protect the Method class from mutations after its instantiation with the method config.

Items left todo:

Make the configured method objects immutable.
Cleanup tests for configuring the Method class and input processing. Make sure I have the following:

Tests for the following:

Method.method_selector_fn()

Accepts string and callable config.
raises exception for anything else.

Method.get_formatters()

Any falsy formatter_lookup_fn config gets the default formatters.
Any non-falsy formatter looup function returns expected formatters.

Method.input_munger():

Passing parameters to the method get passed to mungers expecting the
same parameters.
A falsy mungers config results in use of the default mungers.

Method.process_params():
process_params orchestrates the mungers, method_lookup and formatters so there
should be tests for the following cases:

No mungers, no lookup config, no formatters -> exception
No mungers, a string lookup config, no formatters.
a. with params --> exception
b. no params --> method str, no request params
No mungers, a method lookup fn, no formatters -> method matches param
Several mungers defined, a method str, no formatters -> expected req params
no mungers, a method str, several formatters defined
a. vary the method str -> correct formatter selected
b. check expected output
A comprehensive test.

dylanjw · 2018-12-20T22:06:29Z

@pipermerriam @kclowes @carver This is ready for review.

For the next PR I plan on working on another module that requires more input processing and formatters taken from middlewares.

carver · 2018-12-22T01:13:26Z

Sorry, with the holidays, it will take a while before it is reviewed. Maybe early January (for me at least).

dylanjw · 2018-12-22T20:57:23Z

NP. In the meantime I will start on another PR to work out how migrating the middleware formatters will look, and backmerge any changes from this PR.

pipermerriam · 2019-01-07T22:09:52Z

web3/method.py

+    formatting.  Any processing on the input parameters that need to happen before
+    json_rpc method string selection occurs.
+
+            A note about mungers: The first munger should reflect the desired


This note would be a lot clearer were it to contain a code example.

pipermerriam · 2019-01-07T22:12:00Z

web3/method.py

+    3. request and response formatters are retrieved - formatters are retrieved
+    using the json rpc method string. The lookup function provided by the
+    formatter_lookup_fn configuration is passed the method string and is
+    expected to return a 2 length tuple of lists containing the


nitpick

My vocabulary for this would be 2 length tuple -> 2-tuple but I'm not sure if that nomenclature is well known.

pipermerriam · 2019-01-07T22:13:24Z

web3/method.py

+        return obj.retrieve_caller_fn(self)
+
+    def __setattr__(self, key, val):
+        raise TypeError('Method cannot be modified after instantiation')


I'm leaning towards 👎 on this. trying to enforce immutability on objects seems like a deep rabbit hole.

As long as it's not trying to make a security guarantee, it's simply nudging users toward ideal behavior, then I'm more open to this kind of thing.

pipermerriam · 2019-01-07T22:14:05Z

web3/method.py

+    """
+    _config = None
+
+    def __init__(self, method_config, web3=None):


I'm curious at the use of a dictionary instead of just having individual parameters for each of the various parts that a Method needs. I'm preferential towards the later.

pipermerriam · 2019-01-07T22:22:45Z

web3/method.py

+                if i == 0:
+                    vals = munger(*args, **kwargs)
+                else:
+                    vals = munger(*vals)


Maybe we can split this up to be a bit cleaner.

mungers_iter = iter(mungers) root_munger = first(mungers_iter) munged_inputs = pipe(root_munger(*args, **kwargs), *mungers)

Code above doesn't take into account that each munger needs to be called as munger(*args). One way to deal with this would be to just adjust the API so that each munger is called as munger(args) or maybe a small helper which did the *args application that you wrap each munger function in.

def star_apply(fn): @functools.wraps(fn) def inner(args): return fn(*args) return inner

which conversts the above code to:

mungers_iter = iter(mungers) root_munger = first(mungers_iter) munged_inputs = pipe(root_munger(*args, **kwargs), *map(star_apply, mungers))

pipermerriam · 2019-01-07T22:24:06Z

web3/method.py

+
+
+@to_tuple
+def pipe_and_accumulate(val, fns):


Potentially add an _ prefix to denote private API. (or move it to live under web3._utils

web3/method.py

+        super().__setattr__('_config', dict(method_config))
+
+    def __get__(self, obj=None, objType=None):
+        return obj.retrieve_caller_fn(self)


pipermerriam · 2019-01-07T22:27:43Z

web3/method.py

+    def __init__(self, method_config, web3=None):
+        super().__setattr__('_config', dict(method_config))
+
+    def __get__(self, obj=None, objType=None):


Can you convert to klass or obj_type or typ to stick with python naming conventions.

dylanjw · 2019-01-14T01:01:24Z

Ive added changes to give the mungers access to the module instance, to support certain features, like the default account api.

I didnt want to expose the entire web3 instance to the mungers, so I I created a new Module class to eventually replace it (ModuleV2) that isolates the web3 instance to the method calling functions, rather than exposing a web3 instance to the module. The method callers pass the module instance to the method input processing.

Organizing it this way makes it hard to write "re-entrant" calls at the module level. If I come across any web3 calls in the modules as Im working through the conversion, they will be moved to a middleware, where they can be written for both blocking and async execution pathways.

pipermerriam · 2019-01-14T15:50:06Z

web3/method.py

-    def inner(args):
-        return fn(*args)
+    def inner(module_args):
+        module, args = module_args


The current API of having all the mungers return the module and then pass it onto the next seems problematic/error prone. Instead of requiring the munger to both be accepted as an argument as well I think we should do the following.

only pass as argument.

use curry or functools.partial to set the module parameter on all the munger functions before passing the values through.

This would make the map(star_apply, mungers) into something like:

map(lambda munger: star_apply(functools.partial(munger, module)), mungers)

pipermerriam · 2019-01-14T15:51:30Z

web3/method.py

    if not args and not kwargs:
-        return list()
+        return module, list()


Is the list() intentional here or should this be converted to tuple

pipermerriam · 2019-01-14T15:56:09Z

.localvimrc

@@ -0,0 +1,3 @@
+let test#python#pytest#options = {
+  \ 'suite':   ' tests/core',


This looks like it leaked in and should be removed.

pipermerriam · 2019-01-14T15:59:46Z

tests/core/version-module/test_version_module.py

+@pytest.mark.asyncio
+async def test_async_blocking_version(async_w3, blocking_w3):
+    #  This seems a little awkward.  How do we know if something is an awaitable
+    #  or a static method?


I think this shouldn't be too big a deal, though I might advocate for us ensuring that every one of our convenience properties has a method version as well.

class Version(Module): @property def api(self): return self.get_version() def get_version(self): ...

Reason being that while awaitable properties work just fine, they are not a very intuitive API where-as their method equivalents don't suffer from this awkwardness. It will be up to the user to view the documentation for the various methods and to know that they are coroutines.

pipermerriam · 2019-01-14T16:03:28Z

web3/version.py

+    _get_protocol_version = Method('eth_protocolVersion')
+
+    @property
+    def api(self):


I think I'm 👍 on having a mixed-bag of coroutine and normal properties. Forcing simple APIs like this that don't require I/O to be coroutines makes them more complex than necessary and adds un-necessary performance overhead. 👍

dylanjw · 2019-01-15T08:09:10Z

web3/manager.py

+            self.web3,
+            tuple(self.middleware_stack))
+        self.logger.debug("Making request. Method: %s", method)
+        return request_func(method, params)


@kclowes @njgheorghita
Before I merge this, it would be good to get a couple sets of eyes on this. Its the only place in this PR that touches the critical api.

With no longer iterating over the provider list, I think the UnhandledRequest exception is unnecessary and we can let the CannotHandleRequest pass through. Or maybe this should be catching the "CannotHandleRequest" error like before?

For reference here is the request function when it had the provider list so you dont have to dig through the history:

# # Provider requests and response # def _make_request(self, method, params): for provider in self.providers: request_func = provider.request_func(self.web3, tuple(self.middleware_stack)) self.logger.debug("Making request. Method: %s", method) try: return request_func(method, params) except CannotHandleRequest: continue else: raise UnhandledRequest( "No providers responded to the RPC request:\n" "method:{0}\n" "params:{1}\n".format( method, params, ) )

From what I understand, it looks good to me. I'm thinking it's fine to let the CannotHandleRequest exception pass through from the provider rather than catching it here

Though, maybe beefing up the msg here to have more info like in the UnhandledRequest msg from before could be useful

Looks correct.

If CannotHandleRequest is no longer used anywhere it can be removed (didn't look to see if you already did this)

Yeah, I agree with @njgheorghita. CannotHandleRequest makes sense to me, especially with a better error message!

- Allows method configuration to be made in one place. - Blocking and Async execution pathways can be controlled by the module, using the is_async attribute.

adiochen1 · 2021-01-02T09:33:42Z

Hey, does web3 currently support async? How do I initiate HTTPProvider Web3 client in an async-matter?

kclowes · 2021-01-06T16:58:02Z

@adichen3798 It does not yet support async, but the goal is to get something out by the end of the quarter!

Nighty13 · 2021-05-17T14:27:18Z

Hello , still no support for async?

cosmikwolf · 2021-05-27T18:58:38Z

looks like async web3 is in development here: https://github.com/guanqun/async-web3.py

kclowes · 2021-05-27T19:31:28Z

Nice! Async is a work in progress for us but is actively being worked on. This issue is the most up to date if you're interested in following along.

dylanjw force-pushed the async_api branch from 1e4a021 to 97adc5f Compare December 15, 2018 01:26

dylanjw commented Dec 15, 2018

View reviewed changes

pipermerriam reviewed Dec 17, 2018

View reviewed changes

pipermerriam reviewed Jan 7, 2019

View reviewed changes

dylanjw force-pushed the async_api branch 4 times, most recently from 2815866 to 8ca6573 Compare January 13, 2019 08:34

pipermerriam reviewed Jan 14, 2019

View reviewed changes

pipermerriam approved these changes Jan 14, 2019

View reviewed changes

dylanjw force-pushed the async_api branch 2 times, most recently from 8f264c1 to 486e28f Compare January 15, 2019 07:36

dylanjw commented Jan 15, 2019

View reviewed changes

dylanjw mentioned this pull request Jan 15, 2019

Migrate formatting middlewares to Method formatters #1209

Closed

5 tasks

pipermerriam mentioned this pull request Jan 15, 2019

Relocate personal RPC endpoints to parity and geth class #1211

Merged

dylanjw added 3 commits January 16, 2019 17:49

Create async manager and test provider request function

699bb51

Add Method() class

de9635b

- Allows method configuration to be made in one place. - Blocking and Async execution pathways can be controlled by the module, using the is_async attribute.

Demo async/blocking module definition with web3.Version

5903c06

dylanjw force-pushed the async_api branch from 6be6489 to 5903c06 Compare January 16, 2019 09:49

Add more detail to CannotHandleRequest message

2cd99c6

dylanjw merged commit e2763c5 into ethereum:master Jan 17, 2019

kclowes added this to the Async Web3 API milestone Aug 7, 2019

mikeshultz mentioned this pull request Sep 12, 2020

Docs still mention UnhandledRequest but it's been removed #1737

Closed

		@@ -0,0 +1,3 @@
		let test#python#pytest#options = {
		\ 'suite': ' tests/core',

outline for async web3 api: implement async Version module #1166

outline for async web3 api: implement async Version module #1166

Conversation

dylanjw commented Dec 15, 2018 • edited Loading

Cute Animal Picture

dylanjw Dec 15, 2018 • 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

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

dylanjw commented Dec 19, 2018 • edited Loading

dylanjw commented Dec 20, 2018

carver commented Dec 22, 2018

dylanjw commented Dec 22, 2018

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

This comment was marked as resolved.

Choose a reason for hiding this comment

dylanjw commented Jan 14, 2019

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

dylanjw Jan 15, 2019 • 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

Choose a reason for hiding this comment

adiochen1 commented Jan 2, 2021

kclowes commented Jan 6, 2021

Nighty13 commented May 17, 2021

cosmikwolf commented May 27, 2021

kclowes commented May 27, 2021

dylanjw commented Dec 15, 2018 •

edited

Loading

dylanjw Dec 15, 2018 •

edited

Loading

dylanjw commented Dec 19, 2018 •

edited

Loading

dylanjw Jan 15, 2019 •

edited

Loading