Improve handling of string type and non-attribute template_fields

[josh-fell]

Related: #19047, #19052
Closes: #12876

This PR intends to improve both the handling of template_fields values declared as strings (most typically seen as template_fields = ("some_field") as well as the error message for declaring template_fields that are not attributes of the operator. In the first case, a warning is logged and the template_fields is "automagically" converted to a list for the task execution. For the second case, the error message is updated from the default AttributeError to something more specific to what is happening when attempting to render any template fields.

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

[uranusjr]

Suggested change

	f"The `template_fields` value for {self.task_type} is a string "
	"but should be a Sequence type. Converting to a Sequence type for execution. "
	f"Please update {self.task_type} accordingly.",
	f"The `template_fields` value for {self.task_type} is a string "
	"but should be a list of string. Wrapping it in a list for execution. "
	f"Please update {self.task_type} accordingly.",

str is technically a sequence, so sequence of string is a better term. But I feel the term sequence itself might not be the easiest to understand for less experienced developers, so let’s just recommend using a list instead, which is the most fool-proof syntax.

[josh-fell]

Excellent point.

[uranusjr]

Suggested change

	if hasattr(parent, attr_name):
	content = getattr(parent, attr_name)
	if content:
	rendered_content = self.render_template(content, context, jinja_env, seen_oids)
	setattr(parent, attr_name, rendered_content)
	else:
	raise AttributeError(
	f"{attr_name!r} is configured as a template field "
	f"but {parent.task_type} does not have this attribute."
	)
	try:
	content = getattr(parent, attr_name)
	except AttributeError:
	raise AttributeError(
	f"{attr_name!r} is configured as a template field "
	f"but {parent.task_type} does not have this attribute."
	)
	else:
	if content:
	rendered_content = self.render_template(content, context, jinja_env, seen_oids)
	setattr(parent, attr_name, rendered_content)

EAFP over LBYL.

[ashb]

except ... else not needed.

[josh-fell]

Ah yes. More succinct as well.

[uranusjr]

Same as above.

[josh-fell]

Pushed some fixes for static check errors...come on Josh. 🤦

[potiuk]

Pushed some fixes for static check errors...come on Josh. facepalm

Hint:

pre-commit run --from-ref=HEAD^ --to-ref=HEAD

And a sneaky preview of new Breeze2 equivalent:

./Breeze2 static-checks --last-commit

[potiuk]

cc: @Bowrna ^^

[josh-fell]

Pushed some fixes for static check errors...come on Josh. facepalm

Hint:

pre-commit run --from-ref=HEAD^ --to-ref=HEAD

And a sneaky preview of new Breeze2 equivalent:

./Breeze2 static-checks --last-commit

Yeah that's usually my go-to move but I completely spaced on it this time. That new command is lovely. I'll have to try it out.

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

[potiuk]

Very nice. Love the explicit communication!