dstandish commented on PR #35416:
URL: https://github.com/apache/airflow/pull/35416#issuecomment-1793020761

   OK first there was #34127 by @dkim010, which added the "reusing" of the 
PodLogsConsumer object.
   
   But then after that came #34412 by @hussein-awala , which changed the 
handling of the object.
   
   Now, it appears to me, that the forwarding of the logs object does not have 
any effect.
   
   Inside consume_logs, a new consumer object is always created:
   
https://github.com/apache/airflow/pull/35416/files#diff-6900da9281d8404b14da0815f2b37350f3148b1b63449928b952744e6711e7e7L414
   
   If that step fails, nothing is done with the consumer object.  In that 
situation, the old consumer object would be returned along with [last captured 
time](https://github.com/apache/airflow/pull/35416/files#diff-6900da9281d8404b14da0815f2b37350f3148b1b63449928b952744e6711e7e7L467);
 but then after that it would just be at most passed back in to consume_logs 
again, and ignored again.
   
   Do you agree @hussein-awala and @dkim010?   I think this PR is an 
appropriate cleanup.
   
   KPO logging is now extremely convoluted.  One avenue for cleanup is getting 
rid of some irrelevant `follow=` logic, which I'll try to do.  Some of that was 
relevant only when triggers could not log, so we added periodic resuming and 
log capturing, which is not a feature in this operator anymore.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to