SameerMesiah97 commented on code in PR #74920:
URL: https://github.com/apache/airflow/pull/74920#discussion_r4303544498
##########
task-sdk/tests/task_sdk/bases/test_operator.py:
##########
@@ -798,6 +798,49 @@ class StringTemplateFieldsOperator(BaseOperator):
assert len(warnings) == 1
assert isinstance(task.template_fields, list)
+ def
test_template_fields_extra_class_level_merges_into_template_fields(self):
+ """Extras declared on the class appear in ``template_fields`` after
init."""
+
+ class ExtraTemplateOperator(BaseOperator):
+ template_fields = ("arg1",)
+ template_fields_extra = frozenset({"arg2", "arg3"})
+
+ task = ExtraTemplateOperator(task_id="op1", arg1="{{ x }}", arg2="{{ y
}}", arg3="{{ z }}")
+
+ rendered = sorted(task.template_fields)
+ assert rendered == ["arg1", "arg2", "arg3"]
Review Comment:
What about ordering and deduplication? I think you should add args that
exist in both the template fields and its extra. And assert that the intended
order is retained and duplicated args are handled i.e. only unique args exist
in rendered.
##########
task-sdk/tests/task_sdk/bases/test_operator.py:
##########
@@ -798,6 +798,49 @@ class StringTemplateFieldsOperator(BaseOperator):
assert len(warnings) == 1
assert isinstance(task.template_fields, list)
+ def
test_template_fields_extra_class_level_merges_into_template_fields(self):
+ """Extras declared on the class appear in ``template_fields`` after
init."""
+
+ class ExtraTemplateOperator(BaseOperator):
+ template_fields = ("arg1",)
Review Comment:
Does this not need an __init__? Also, since this helper class is being used
multiple times, you could make it module level and reuse it in the tests that
require it.
--
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]