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

Async HTTP Provider #1978

Merged
merged 1 commit into from
May 10, 2021
Merged

Async HTTP Provider #1978

merged 1 commit into from
May 10, 2021

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented May 5, 2021

What was wrong?

Adds a bare bones async http provider. No middleware yet, and no session caching.

Related to Issue #1413

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes changed the title [WIP] Async base provider [WIP] Async HTTP Provider May 5, 2021
@kclowes kclowes requested review from wolovim and pipermerriam May 5, 2021 21:49
@kclowes
Copy link
Collaborator Author

kclowes commented May 5, 2021

I still have some typing to work out, but would love to get some feedback on general shape! This is the MVP implementation with absolutely nothing fancy - no middleware, no session caching, etc. I'll put those features in separate PRs!

@kclowes kclowes force-pushed the async-base-provider branch from 3208e26 to 5a76a89 Compare May 6, 2021 16:44

@pytest.mark.asyncio
async def test_isConnected(self, async_w3_http: "Web3") -> None:
assert await async_w3_http.isConnected() is True
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm getting a mypy error on this line:
Incompatible types in "await" (actual type "bool", expected type "Awaitable[Any]")

And I'm not sure how to fix it. I think it should be a bool, but I'm not sure where/why it thinks it should be an Awaitable[Any] 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think maybe what this line is actually doing is assert await (async_w3_http.isConnected() is True)

The await may not be applied before the is True causing this to be a boolean being fed to an await statement?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe because of the decorator, I guess? Try adding @overload

@kclowes kclowes force-pushed the async-base-provider branch 2 times, most recently from cd2c442 to 3b3f51c Compare May 7, 2021 16:13
Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

👍 structure looks fine (and sane enough).


@pytest.mark.asyncio
async def test_isConnected(self, async_w3_http: "Web3") -> None:
assert await async_w3_http.isConnected() is True
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe what this line is actually doing is assert await (async_w3_http.isConnected() is True)

The await may not be applied before the is True causing this to be a boolean being fed to an await statement?


@property
async def gas_price(self) -> Wei:
# types ignored b/c mypy conflict with BlockingEth properties
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is unfortunate and would be nice to resolve... but I'm not sure what to suggest.

@kclowes kclowes force-pushed the async-base-provider branch from 3b3f51c to 9cee725 Compare May 7, 2021 20:48
@kclowes kclowes changed the title [WIP] Async HTTP Provider Async HTTP Provider May 7, 2021
Copy link
Member

@wolovim wolovim left a comment

Choose a reason for hiding this comment

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

Looks like a great start! 🚀

A preliminary benchmark:

[sync] 30 eth_gas calls:  2.4563300249999998
[async] 30 eth_gas calls:  0.4444956649999998

Will see about getting a benchmark PR up next week.

@kclowes kclowes force-pushed the async-base-provider branch from f9db8fe to 1a32209 Compare May 10, 2021 17:32
@kclowes kclowes merged commit 63dd970 into ethereum:master May 10, 2021
@kclowes kclowes deleted the async-base-provider branch May 10, 2021 17:53
@kclowes kclowes mentioned this pull request May 10, 2021
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.

4 participants