-
Notifications
You must be signed in to change notification settings - Fork 180
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
[experiment] Add resource time limit and rate limiting #1485
base: devel
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
if max_items >= 0 or max_time and not self.incremental: | ||
from dlt.common import logger | ||
|
||
logger.warning( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will need to be improved a bit, but I think this is a quite nice solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also: generally speaking it would be cool to have some wrapper around log messages to be able to test them, maybe mocking would be enough, not sure
while (last_iteration + min_wait) - time.time() > 0: | ||
# we give control back to the pipe iterator | ||
yield None | ||
time.sleep(0.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could make this configurable, I am not sure wether it is needed though.
@@ -788,52 +788,6 @@ def test_add_transformer_right_pipe() -> None: | |||
iter([1, 2, 3]) | dlt.resource(lambda i: i * 3, name="lambda") | |||
|
|||
|
|||
def test_limit_infinite_counter() -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these three tests were moved to the new location where a couple more tests will be added specifically for the limits
2ee3eab
to
e48f641
Compare
6126895
to
5fc2324
Compare
Description
This PR extends the add_limit function to add time limits and rate limits to resources. This approach is to be discussed, but very straightforward, easy to test and works for both sync and async resources.
TODOs (if we go this route):
Other thoughts: