Add API rate limit handler #371
Conversation
src/neo4j_graphrag/llm/rate_limit.py
Outdated
| raise convert_to_rate_limit_error(e) | ||
| raise | ||
|
|
||
| return active_handler.handle_sync(inner_func)() |
There was a problem hiding this comment.
Do we need to call the returned function here?
There was a problem hiding this comment.
I am not so familiar with defining decorators. But don't we still need this to return the retry version of the inner function?
There was a problem hiding this comment.
The decorator should return a function, that's why I tend to think the final () are not needed, but I haven't tested.
There was a problem hiding this comment.
If we omit the trailing (), the decorator would return the wrapped function object instead of the function’s result.
There was a problem hiding this comment.
Ok, then shouldn't the return type of rate_limit_handler by Any instead of F?
There was a problem hiding this comment.
but doesn't using F preserve more type safety since at least at call sites the method has the same type as it has before decoration?
… provider classes
NathalieCharbel
force-pushed
the
fix-api-rate-limit-issues
branch
from
8cfacb4 to
af3a5d2
Compare
Description
RateLimitHandlerinterfaceRateLimitErrorRateLimitHandlerinterfaceType of Change
Complexity
Complexity: Medium
How Has This Been Tested?
Checklist
The following requirements should have been met (depending on the changes in the branch):