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

Clean up AsyncHTTPProvider instantiation #2736

Merged

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Nov 23, 2022

What was wrong?

Related to #1416. This is listed as non-breaking but instead of having an async_net module, it seems more appropriate to set the net module up as the AsyncNet class. This will break things.

How was it fixed?

  • Add is_async flag to base provider classes and appropriately set them to True or False.
  • Use these flags to determine whether to load sync or async default middlewares and default modules.
  • Refactor the Web3 property async_net as net mapped to AsyncNet class when dealing with async and Net when dealing with sync.

Todo:

Cute Animal Picture

___ O>
/\ /\

@fselmo fselmo force-pushed the clean-up-async-provider-instantiation branch 3 times, most recently from a09f5fd to 955e539 Compare November 23, 2022 00:34
@fselmo fselmo mentioned this pull request Nov 23, 2022
22 tasks
- Add ``is_async`` flag to base provider classes and appropriately set them to ``True`` or ``False``.
- Use these flags to determine whether to load sync or async default middlewares and default modules.
@fselmo fselmo force-pushed the clean-up-async-provider-instantiation branch 4 times, most recently from c0cc52d to d4f6fad Compare November 29, 2022 00:57
- Also minor cleanup that came along with these changes: e.g. expect a more specific error, ``NameNotFound`` for ens test since the async version started to wrongly xpass.
@fselmo fselmo force-pushed the clean-up-async-provider-instantiation branch from d4f6fad to 7882513 Compare November 29, 2022 01:16
@fselmo fselmo marked this pull request as ready for review November 29, 2022 01:24
@fselmo fselmo force-pushed the clean-up-async-provider-instantiation branch from 7882513 to e087a46 Compare November 29, 2022 01:37


def test_web3_with_async_http_provider_has_default_middlewares_and_modules() -> None:
async_w3 = Web3(AsyncHTTPProvider(endpoint_uri="http://mynode.local:8545"))
Copy link
Contributor

Choose a reason for hiding this comment

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

can use URI here

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.

one small update noted, otherwise lgtm! thanks!

- No need for the ``Net`` module to be the only one to have to `cast` values because the `net` property on `Web3` is a `Union[Net, AsyncNet]`. Instead, set it like the others and make one decision for all at the same time in an upcoming PR.
@fselmo fselmo force-pushed the clean-up-async-provider-instantiation branch from 4231169 to f06e3cd Compare December 2, 2022 01:23
@fselmo fselmo merged commit 1b5d7ae into ethereum:master Dec 2, 2022
fselmo added a commit that referenced this pull request Dec 2, 2022
@fselmo fselmo deleted the clean-up-async-provider-instantiation branch April 3, 2024 20:51
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.

2 participants