Skip to content

Conversation

@lrusak
Copy link
Contributor

@lrusak lrusak commented Oct 3, 2024

Hello! I'm going to make a few pull request for some stuff I've been working on.

This fixes an issue I was having when using the async_beacon

What was wrong?

Traceback (most recent call last):
  File "/home/lukas/Documents/git/web3.py/test-script.py", line 33, in <module>
    loop.run_until_complete(main())
  File "/usr/lib64/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/test-script.py", line 22, in main
    validator = await beacon.get_validator(str(validator_id))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/web3/beacon/async_beacon.py", line 114, in get_validator
    return await self._async_make_get_request(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/web3/beacon/async_beacon.py", line 77, in _async_make_get_request
    return await self._request_session_manager.async_json_make_get_request(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/web3/_utils/http_session_manager.py", line 248, in async_json_make_get_request
    response = await self.async_get_response_from_get_request(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/web3/_utils/http_session_manager.py", line 239, in async_get_response_from_get_request
    session = await self.async_cache_and_return_session(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lukas/Documents/git/web3.py/web3/_utils/http_session_manager.py", line 228, in async_cache_and_return_session
    request_timeout.total or DEFAULT_HTTP_TIMEOUT + 0.1,
    ^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'float' object has no attribute 'total'

How was it fixed?

by wrapping the request_timeout parameter with aiohttp.ClientTimeout

Todo:

More info

I'm able to trigger this issue with this example script

import asyncio

from web3.beacon import AsyncBeacon

import logging

import sys

async def main():

    beacon = AsyncBeacon("http://192.168.1.112:5052")

    beacon._request_session_manager.logger.setLevel(logging.DEBUG)

    handler = logging.StreamHandler(stream=sys.stdout)
    handler.setLevel(logging.DEBUG)

    beacon._request_session_manager.logger.addHandler(handler)

    for validator_id in range(1, 500, 1):
        validator = await beacon.get_validator(str(validator_id))

        print(f"{validator_id}: {validator}")

        await asyncio.sleep(0.5)

if __name__ == "__main__":

    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)

    loop.run_until_complete(main())

reedsa pushed a commit to lrusak/web3.py that referenced this pull request Oct 4, 2024
@reedsa
Copy link
Contributor

reedsa commented Oct 4, 2024

@lrusak Thanks for your contributions! This looks good to me but please add tests to check the sync/async request_timeout attribute is initialized with the ClientTimeout instance. I've added a newsfragment so we should be able to merge once the tests are ready!

@reedsa reedsa requested review from fselmo, kclowes and pacrob October 4, 2024 20:19
@lrusak
Copy link
Contributor Author

lrusak commented Oct 8, 2024

@reedsa I pushed a fixup to add tests for the timeout instance. I also reverted the change to the Beacon class because after inspection of web3/_utils/http_session_manager.py it looks like the request_timeout member should be float.

Let me know if that's acceptable and I can adjust the newsfragment also.

@lrusak lrusak changed the title beacon: fix AttributeError by using aiohttp.ClientTimeout throughout AsyncBeacon: fix AttributeError by using aiohttp.ClientTimeout throughout Oct 8, 2024
lrusak pushed a commit to lrusak/web3.py that referenced this pull request Oct 12, 2024
@lrusak lrusak force-pushed the beacon-aiohttp-client-timeout branch from f89f2b3 to 7fb0db8 Compare October 12, 2024 20:20
lrusak pushed a commit to lrusak/web3.py that referenced this pull request Oct 12, 2024
@lrusak lrusak force-pushed the beacon-aiohttp-client-timeout branch from 7fb0db8 to f1ac80f Compare October 12, 2024 20:40
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.

Looks great, thanks!

…imeout`` for requests.

- ``HTTPSessionManager`` expects the timeout kwarg to be of instance ``aiohttp.ClientTimeout``
- Newsfragment for ethereum#3503

Signed-off-by: Lukas Rusak <[email protected]>
@fselmo fselmo force-pushed the beacon-aiohttp-client-timeout branch from f1ac80f to 419b098 Compare October 12, 2024 21:29
@fselmo fselmo merged commit fddf9de into ethereum:main Oct 12, 2024
55 of 71 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.

3 participants