shunping commented on PR #35373: URL: https://github.com/apache/beam/pull/35373#issuecomment-3074702217
> In my opinion, for retry_strategy=RETRY_NEVER, there will be only one condition i.e. RETRY_NEVER + any with should_retry= false, so there is no harm in logging ERROR here. This means whatever error we are not retrying, it is worth highlighting that message as ERROR once. > > @claudevdm can give the final lgtm. I think your current implementation mostly agrees with the table at https://github.com/apache/beam/pull/35373#issuecomment-3071605810. In that case, why don't we simplify the following condition? ``` if (should_retry and self._retry_strategy in [RetryStrategy.RETRY_ON_TRANSIENT_ERROR, RetryStrategy.RETRY_ALL): log_level = logging.WARN else: log_level = logging.ERROR ``` -- 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: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
