Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add AsyncIPCProvider #2984

Merged
merged 13 commits into from
Feb 23, 2024
Merged

✨ Add AsyncIPCProvider #2984

merged 13 commits into from
Feb 23, 2024

Conversation

DavidRomanovizc
Copy link
Contributor

@DavidRomanovizc DavidRomanovizc commented Jun 7, 2023

What was wrong?

IPCProvider needs async version

Related to Issue #2973
Closes #1413

How was it fixed?

Add AsyncIPCProvider

Todo:

Cute Animal Picture

image

web3/providers/async_ipc.py Outdated Show resolved Hide resolved
web3/providers/async_ipc.py Outdated Show resolved Hide resolved
@JustAnotherC0der
Copy link

Pardon my ignorance to github work flow but is this implemented yet? Can't seem to import AsyncIPCProvider on newest web3

@kclowes
Copy link
Collaborator

kclowes commented Jul 31, 2023

@JustAnotherC0der It's not yet. We're still working on it.

@kclowes
Copy link
Collaborator

kclowes commented Dec 20, 2023

This is ready for review (finally)! I will add docs tomorrow, but wanted to get the code out there for feedback!

@SheldonHolmgren
Copy link

Thanks for your hard work. I will really appreciate this feature.

Any plans to make AsyncIPCProvider a PersistentConnectionProvider? I think without this it will be impossible to w3.eth.subscribe():


Web3ValidationError                       Traceback (most recent call last)
<ipython-input-12-2fc2efbc61b6> in <module>
----> 1 async with AsyncWeb3.persistent_websocket(AsyncIPCProvider("/mnt/ssd/optimism_data/geth.ipc")) as w3:
      2     await w3.eth.subscribe("newHeads")

~/dev/web3.py/web3/main.py in persistent_websocket(provider, middlewares, modules, external_modules, ens)
    516         a ``PersistentConnectionProvider`` instance.
    517         """
--> 518         return _PersistentConnectionWeb3(
    519             provider,
    520             middlewares,

~/dev/web3.py/web3/main.py in __init__(self, provider, middlewares, modules, external_modules, ens)
    541     ) -> None:
    542         if not isinstance(provider, PersistentConnectionProvider):
--> 543             raise Web3ValidationError(
    544                 "Provider must inherit from PersistentConnectionProvider class."

This is not a big deal since I can still use filters to get similar functionality.

Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for everyone's contribution here. I think this will be a great option to have. I left some initial comments. Mostly curious about the PersistentConnectionProvider question.

web3/tools/benchmark/main.py Outdated Show resolved Hide resolved
web3/providers/async_ipc.py Outdated Show resolved Hide resolved
web3/providers/async_ipc.py Outdated Show resolved Hide resolved
web3/providers/async_ipc.py Outdated Show resolved Hide resolved
web3/providers/async_ipc.py Outdated Show resolved Hide resolved
@SheldonHolmgren
Copy link

SheldonHolmgren commented Feb 13, 2024

This code won't work if the client receives multiple responses before it reads the first one. I have coded up a dirty fix for my own use. Perhaps it might be useful as a hint: https://github.com/DavidRomanovizc/web3.py/compare/main...SheldonHolmgren:async_ipc_multiple_resps

@kclowes
Copy link
Collaborator

kclowes commented Feb 15, 2024

Thank you @SheldonHolmgren! I'll see what I can do.

@kclowes
Copy link
Collaborator

kclowes commented Feb 21, 2024

All right, I think this is ready again for another look. Thanks @fselmo for the boost!

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I caught one minor doc change we missed but that was all I saw (and I went ahead and pushed it). Thanks for all the work here @DavidRomanovizc, @kclowes, and @SheldonHolmgren! lgtm 👍🏼

@kclowes I'll let you merge this in when you are good with it. Maybe some more squashing if it makes sense too?

DavidRomanovizc and others added 9 commits February 22, 2024 12:19
- Iron out some issues with rebasing; update async_ipc
- Time subscription tests correctly
- Fix lint
- Fix doc underline
- Remove sleeps in test to test multiple responses coming in quick succession
- Handle when multiple responses come in at once
- Add test for subscription
- timeout -> request_timeout

Updates to naming:
  - Remove ws specific naming from manager methods
  - _ws_message_listener -> _message_listener
  - WebsocketConnection -> PersistentConnection
  - persistent_websocket -> persistent_connection
- Clean up ``AsyncIPCProvider``. Some logging c/p was left in the code.
- Remove the need for the ``PersistentSocket`` class.
- Move the ``silence_listener_task_exceptions`` setting to the base class,
  ``PersistentConnectionProvider``.
- Use the suggestion put forth in the PR #2984 for parsing messages from the
  IPC socket more efficiently in case more than one message is received at
  once.
- Update docs for persistent connection provider refactor

Remove sleeps to emulate responses in rapid succession
web3/manager.py Outdated

async def _ws_message_stream(self) -> AsyncGenerator[RPCResponse, None]:
async def _message_stream(self) -> AsyncGenerator[RPCResponse, None]:
if not isinstance(self._provider, PersistentConnectionProvider):
raise TypeError(
"Only websocket providers that maintain an open, persistent connection "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we're removing references to websockets, and either just using "socket" or "persistent connection". Should that happen here too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oop, yep. Nice catch!

@@ -261,7 +376,8 @@ Persistent Connection Providers

.. py:class:: web3.providers.persistent.PersistentConnectionProvider(endpoint_uri: str, request_timeout: float = 50.0, subscription_response_queue_size: int = 500)

This is a base provider class, currently inherited by the ``WebsocketProviderV2``.
This is a base provider class, currently inherited by the ``WebsocketProviderV2``,
and the ``AsyncIPCProvider``.
It handles interactions with a persistent connection to a JSON-RPC server. Among
its configuration, it houses all of the
:class:`~web3.providers.websocket.request_processor.RequestProcessor` logic for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This moved to web3.providers.persistent.request_processor I believe.

@kclowes kclowes merged commit a02516c into ethereum:main Feb 23, 2024
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web3 Async API Tracking Issue
6 participants