korex-f commented on code in PR #66633:
URL: https://github.com/apache/airflow/pull/66633#discussion_r3328318080
##########
providers/amazon/src/airflow/providers/amazon/aws/log/cloudwatch_task_handler.py:
##########
@@ -118,16 +119,38 @@ def handler(self) -> watchtower.CloudWatchLogHandler:
json_serialize_default=_json_serialize or json_serialize_legacy,
)
+ _handler_creating: bool = attrs.field(default=False, init=False,
repr=False)
+
+ @property
+ def handler(self) -> watchtower.CloudWatchLogHandler:
+ if self._handler_creating:
+ # Re-entrant call during handler creation, some libraries log
internally
+ # during initialization, which triggers this process again before
+ # handler is fully constructed.
+ if self._handler is not None and isinstance(self._handler,
watchtower.CloudWatchLogHandler):
+ return self._handler
+ raise structlog.DropEvent()
+ if self._handler is None or self._handler.shutting_down:
+ self._handler_creating = True
+ try:
+ self._handler = self._create_handler()
+ finally:
+ self._handler_creating = False
+ return self._handler
+
@cached_property
def processors(self) -> tuple[structlog.typing.Processor, ...]:
from logging import getLogRecordFactory
import structlog.stdlib
logRecordFactory = getLogRecordFactory()
- # The handler MUST be initted here, before the processor is actually
used to log anything.
- # Otherwise, logging that occurs during the creation of the handler
can create infinite loops.
- _handler = self.handler
+ # Eagerly initialize the handler before the closure is created.
+ # Without this, the first log emission would trigger handler creation,
+ # which itself logs, causing an infinite loop.
+ self._handler
Review Comment:
> I could be mistaken, but shouldn't this be hitting the new `self.handler`
property instead of the underlying `self._handler` field to make sure it gets
init'd?
You're absolutely right, that's a bug. self._handler directly accesses the
backing field which is None at that point, so the eager init is currently a
no-op and the re-entrancy guard never fires during initialization. I would
change this to self.handler now. Thanks for catching this.
--
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]
