[Bug]: Use a unique token for log pagination instead of a timestamp#2845
[Bug]: Use a unique token for log pagination instead of a timestamp#2845peterschmidt85 merged 1 commit intomasterfrom
Conversation
| except LogStorageError: | ||
| # Group doesn't exist, re-raise the LogStorageError | ||
| raise |
There was a problem hiding this comment.
Why catching and re-raising immediately?
There was a problem hiding this comment.
Yeah, it looks a bit odd but the idea is to catch a abstract exception and re-raise only if indeed streams do not exist.
This allows to heck stream existence only if error occurs.
There was a problem hiding this comment.
You catch and re-raise LogStorageError, which is basically the same as not catching it in the first place.
| # We intentionally make reading logs slow to prevent hitting GCP quota. | ||
| # This doesn't help with many concurrent clients but | ||
| # should help with one client reading all logs sequentially. | ||
| time.sleep(1) |
There was a problem hiding this comment.
Without it, you'd hit quota and wouldn't be able to read the logs at all.
There was a problem hiding this comment.
Yeah. Do you think we can raise a specific exception and handle it on the client?
There was a problem hiding this comment.
We already return an error when the quota is hit, see above. At that point, the client can do very little – you may have to wait for a minute or so. So we prevent quota hit in the first place.
2 participants
Fixes #2832, #2833, and potentially #2783
Here's what this PR does:
[Major]
next_tokenproperty in/logs/pollnext_tokennext_tokento load paginated logs (this fixes [Bug]: Can't see the entire log of a run via UI #2832)[Minor]
Here's what this PR doesn't solve: