dstandish commented on PR #36227: URL: https://github.com/apache/airflow/pull/36227#issuecomment-1861190610
> I agree that the test doesn't cover this check, but IMHO, we need to update the test and not the code to aggregate the log lines if they are really split for some reason. Like I tried to indicate, my goal was to not necessarily to revert your change. But I wanted to understand what your change was trying to do. And one way to do that is to revert it and use the test to see expected behavior vs actual. I agree it would be best if you can update the tests with the right "expected" cases that really reveal what it's supposed to do, both with respect to `log.info` and `progress_callback`. This is important because otherwise any future refactor could easily unknowingly remove your change. > Just to make sure it was intentional, you didn't revert all the changes, where you removed the statement that logs the error message: It was nontrivial to apply the revert patch, and I may not have gotten everything 100% -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
