dstandish commented on code in PR #40468:
URL: https://github.com/apache/airflow/pull/40468#discussion_r1664855333
##########
airflow/executors/base_executor.py:
##########
@@ -122,6 +123,7 @@ class BaseExecutor(LoggingMixin):
job_id: None | int | str = None
name: None | ExecutorName = None
callback_sink: BaseCallbackSink | None = None
+ task_context_logger: TaskContextLogger
Review Comment:
> Are you sure about this? TaskContextLogger is currently being used int eh
scheduler with a component name static, thus, if you're right, that'd mean that
scheduler logs override each other? Or I misunderstood what you said.
https://github.com/apache/airflow/blob/main/airflow/jobs/scheduler_job_runner.py#L240
Yep, pretty sure. But there may be misunderstanding also. So, the
component gets added to the end of the full log name (which includes normal
task log filename) so there would only be a collision if there were two
exceptional events for the same task instance and same try number. Pretty
unlikely I'm sure, and perhaps logically impossible currently with the events
that are logged. But, still, we should just add a random string to the suffix
to make it truly impossible. (note: not suggesting in this PR) Because more
important as more events are logged. I guess it might not be a change to the
params or anything just change in behavior.
--
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]
