-
Notifications
You must be signed in to change notification settings - Fork 765
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
fix: missing loglines from tailing logs with GCP #2112
Conversation
@@ -710,12 +710,23 @@ def wait_for_launch(job): | |||
wait_for_launch(self._job) | |||
|
|||
# 2) Tail logs until the job has finished | |||
self._output_final_logs = False |
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.
can we use generator -
def _has_updates():
while self._job.is_running:
yield True
yield True # One final update after job finishes
yield False
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.
yup this should work too, and it reduces reliance on the other status internals. I'll make the changes
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.
scratch that, it won't work without larger changes as the tail_logs
has_log_updates=
callable is simply used in a while loop, which won't exhaust the generator (it counts as true always)
is there any perf impact? |
the tailing happens client-side so no performance impact on the actual task execution code during remote exec. |
Fixes an issue where log tailing stops working after a point on GCP, resulting in missing log output on the terminal.
The cause is with the behaviour of
google.cloud.storage
blob client, where consecutive requests with identical attributes on a single instance will not yield different responses, even if the underlying blob has advanced.Example flow to reproduce missing log lines on GCP:
also small fix to Azure log tailing as well.