feature: callable for template_fields

[raphaelauv]

following discussion of #35844

revert templating from #31362

[dirrao]

Can you add the test cases for the change?

[uranusjr]

I wonder if we should turn the return value of the callable into a template and render that. Since a callable can do whatever it wants, supporting returning a template seems redundant and may induce some confusing behaviours (the same as template_fields already does in some edge cases).

As a side note, I wonder if the callable should take context as an argument.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

[uranusjr]

Yeah after some more thought I really do not like the fact that the return value gets treated as a template string. The callable should just return the rendered value instead. This is what we do with XComArg—a resolved XCom value is treated as-is, not rendered again as a template.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

[raphaelauv]

no stale

[raphaelauv]

@uranusjr thanks for your review , wdyt of this second implementation ?

[uranusjr]

The idea looks fine to me, but this should be a part of _do_render_template_fields instead, or even somewhere deeper in the call chain. render_template_fields is an override point for users and we shouldn’t put logic in there where people can shadow.

I wonder if we should make this a part of render_template.

[raphaelauv]

@uranusjr thanks again, I did the change

[uranusjr]

Also we need documentation on this.

[raphaelauv]

Also we need documentation on this.

yeah I will when we find the final implementation

[uranusjr]

Design looks good now to me.

[raphaelauv]

@uranusjr could you pls review the doc commit , thanks

[uranusjr]

I touched up the documentation so the full document reads better. Looks good to me now. Does this work with nested fields?

PythonOperator(
    ...,
    op_kwargs={"foo": render_foo},  # Is this callable rendered?
)

We probably should not allow this since it would be a bit confusing.

[raphaelauv]

thanks for the doc , I added a new test about nested fields

[potiuk]

Very good idea and implementation.