dstandish commented on code in PR #40468:
URL: https://github.com/apache/airflow/pull/40468#discussion_r1664452613
##########
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:
> I am not sure this is a huge risk to make it public, the only public
methods of this class are the emitting log methods such as error, info, ... The
signatures of these methods are pretty generic and the risk to have a breaking
change is pretty low?
Yeah the other part of what would constitute "the interface" would be i
suppose like the params `def __init__(self, component_name: str,
call_site_logger: Logger | None = None):` and such. I am just very wary of
making things public that aren't really for users because it is such an
impediment when we want to change things.
One thing change we need to make with this interface, that I think was
missed, is that each file needs to have a random string in it because,
especially as more usages of this are added, there will be times when multiple
such messages are added for a single TI, and currently, if they are from the
same "component", then they will collide / override.
--
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]
