add __repr__ method for AsyncHTTPConnectionPool & other related classes
#205
Conversation
| @@ -174,6 +174,9 @@ def __init__( | |||
| else resolver | |||
| ) | |||
|
|
|||
| def __repr__(self) -> str: | |||
| return f"<{self.__class__.__name__} num_pools={self._num_pools}>" | |||
There was a problem hiding this comment.
| return f"<{self.__class__.__name__} num_pools={self._num_pools}>" | |
| return f"<{self.__class__.__name__} num_pools={len(self.pools)}>" |
should this be the more preferred approach? since _num_pools appears to be private attribute.
There was a problem hiding this comment.
avoid the call to self.__class__.__name__, we should hardcode it (the name), it will just consume cpu time for no to little impact.
| @@ -436,6 +436,9 @@ def __init__( | |||
| self._keepalive_idle_window = MINIMAL_KEEPALIVE_IDLE_WINDOW | |||
| self._background_monitoring: asyncio.Task | None = None # type: ignore[type-arg] | |||
|
|
|||
| def __repr__(self) -> str: | |||
| return f"<{self.__class__.__name__} host={self.host!r} port={self.port} timeout={self.timeout}>" | |||
There was a problem hiding this comment.
| return f"<{self.__class__.__name__} host={self.host!r} port={self.port} timeout={self.timeout}>" | |
| return f"<{self.__class__.__name__} host={self.host!r} port={self.port} timeout={self.timeout} retries={self.retries!r} happy_eyeballs={self.happy_eyeballs}>" |
should other attributes such as retries and happy_eyeballs be included here? or the way it is as proposed is ok?
There was a problem hiding this comment.
retries seems a bit overwhelming in debug info as it can get pretty complicated.
happy eyeballs on/off is a good info to keep.
|
There a way to make everything simpler. Down the hole, we should start by implementing We want key info that we will then be able to sub repr in PoolManager or ConnectionPool easily. In TrafficPolice we want a quick summary of what we have in a given moment. Like: for idle/saturated, use The repr should hold the lock until the stats is computed (for the sync part). Then for PoolManager, we basically want: For both sync and async. In order:
That should be it. Regards, |
Hello,
I have opened this draft PR as per our discussion in #194
I am not well versed with this codebase but I did attempt to understand and navigate around the codebase to see what I need to cover for this PR and made the necessary changes upto my understanding. I probably missed something. 😄
I have only currently made changes for async public classes.
However, in the linked issue, you said we will need to cover sync counter parts as well, but I'm unsure which ones exactly to cover. So, please guide me on that and I will do the required.
I have run
nox -rs formatandnox -rs lintcommands on my proposed changes before submitting this PR as per the contrinuting guidelines. Please let me know if I missed anything.