Migrate DagFileProcessor.manage_slas to Internal API

[vincbeck]

Migrate DagFileProcessor.manage_slas to Internal API

Closes: #28268

[vincbeck]

cc @potiuk, @mhenc

[Taragolis]

@vincbeck need resolve conflicts

[vincbeck]

Done

[uranusjr]

Passing in a logger object feels very wrong to me.

[vincbeck]

I agree, it feels also very wrong to me but maybe with some context it will help understanding it :).

All methods decorated with @internal_api_call are meant to be called either directly (like today) or through a new component: internal API (see PR here). This decision is based on the the flag database_access_isolation. What we want to achieve here is when the methods are called directly, the behavior should be the same as today, hence the logger should be the same. However, when these methods are called through the internal API, we want the logs to be separated from the current execution. Then, in that case we want a different logger. This new logger is created by the internal API itself, see PR here.

I hope it helps to understand and feel free to give me your thoughts given this context

[uranusjr]

I understand the technical aspects, but it would simply indicate either the decorator should be improved to remove this restriction, or we need a new mechanism (perhaps built upon @internal_api_call that can be used on an instance method, not only static methods.

[vincbeck]

@mhenc what do you think? Meanwhile I'll try to find a in-between solution

[mhenc]

I don't think we could make it work for non-static methods - passing the whole state of the object would be tricky (we want to avoid pickling) and would affect the performance even more.

If the only problem is passing the logger, then I can see few options:

1. Instead of passing the logger, just send the logger name and create the logger inside the method
2. Make logger static (as Vinc suggested in the first place) - we can wrap it into some method to keep self.log available for other methods.
3. I think internal_api_call should also work for class methods, so we could do something like this:
   https://stackoverflow.com/a/8438818

cc: @potiuk

[potiuk]

PR here: https://github.com/aws-mwaa/upstream-to-airflow/pull/3/files

[vincbeck]

Thanks! I'll update this PR accordingly

[potiuk]

Not nice bot.. Closed my PR :D

[vincbeck]

Hahaha, yeah! It is a bossy and rude bot xD

[vincbeck]

PR updated

[potiuk]

(Pending tests passing :) )