feat(gooddata-pipelines): Process workspace backup sequentially #1138
Conversation
benkeanna
requested review from
hkad98,
jaceksan,
lupko and
pcerny
as code owners
benkeanna
force-pushed
the
aben/parallel-gone
branch
3 times, most recently
from
79dbe0e to
ba0d8ab
Compare
| return 0.0 | ||
|
|
||
| def __enter__(self) -> "RateLimiter": | ||
| self.wait_if_needed() |
There was a problem hiding this comment.
I think it would be great if we could use the rate limiter as a decorator instead of context manager... My assumption is that it could be more versatile (for handling the methods from gooddata_sdk) and less verbose (we could wrap functions/methods with @rate_limiter instead of the with ... blocks and the rate limiter would not have to be a part of the instanced object (like the BackupManager). WDYT?
| self, | ||
| calls_per_second: float = 1.0, | ||
| name: Optional[str] = None, | ||
| logger: Optional[Any] = None, |
There was a problem hiding this comment.
You can reuse the LoggerLike protocol from gooddata-pipelines/gooddata_pipelines/logger/logger.py here as the logger type
|
|
||
| self.calls_per_second = calls_per_second | ||
| self.min_interval = 1.0 / calls_per_second | ||
| self.name = name or "RateLimiter" |
There was a problem hiding this comment.
I don't think this is necessary
| from typing import Any, Optional | ||
|
|
||
|
|
||
| class RateLimiter: |
There was a problem hiding this comment.
I think it would be great if this had some tests
| if time_since_last_call < self.min_interval: | ||
| sleep_time = self.min_interval - time_since_last_call | ||
|
|
||
| if self.logger: |
There was a problem hiding this comment.
On a second thought, I'd consider if it's worth logging every wait and if the logger is needed - it has a potential for being super spammy (1000 workspaces, say 3-5 waits per each?)
| self._lock = threading.Lock() | ||
| self._last_call_time = 0.0 | ||
|
|
||
| def wait_if_needed(self) -> float: |
There was a problem hiding this comment.
Why does this method have a return value? If I see it correctly, it is not stored or read anywhere...
benkeanna
force-pushed
the
aben/parallel-gone
branch
2 times, most recently
from
e3020ad to
bfe8f11
Compare
benkeanna
force-pushed
the
aben/parallel-gone
branch
from
bfe8f11 to
1d466f5
Compare
No description provided.