-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Persist DAG and task doc values in TaskFlow API if explicitly set #29399
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2565,58 +2565,49 @@ def noop_pipeline(): | |
|
||
dag = noop_pipeline() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "noop_pipeline" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typical example of improper assertion that is being fixed in this PR. |
||
assert dag.dag_id == "noop_pipeline" | ||
assert dag.fileloc == __file__ | ||
|
||
def test_set_dag_id(self): | ||
"""Test that checks you can set dag_id from decorator.""" | ||
|
||
@dag_decorator("test", default_args=self.DEFAULT_ARGS) | ||
def noop_pipeline(): | ||
@task_decorator | ||
def return_num(num): | ||
return num | ||
|
||
return_num(4) | ||
... | ||
|
||
dag = noop_pipeline() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "test" | ||
assert dag.dag_id == "test" | ||
|
||
def test_default_dag_id(self): | ||
"""Test that @dag uses function name as default dag id.""" | ||
|
||
@dag_decorator(default_args=self.DEFAULT_ARGS) | ||
def noop_pipeline(): | ||
@task_decorator | ||
def return_num(num): | ||
return num | ||
|
||
return_num(4) | ||
... | ||
|
||
dag = noop_pipeline() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "noop_pipeline" | ||
assert dag.dag_id == "noop_pipeline" | ||
|
||
def test_documentation_added(self): | ||
"""Test that @dag uses function docs as doc_md for DAG object""" | ||
@pytest.mark.parametrize( | ||
argnames=["dag_doc_md", "expected_doc_md"], | ||
argvalues=[ | ||
pytest.param("dag docs.", "dag docs.", id="use_dag_doc_md"), | ||
pytest.param(None, "Regular DAG documentation", id="use_dag_docstring"), | ||
], | ||
) | ||
def test_documentation_added(self, dag_doc_md, expected_doc_md): | ||
"""Test that @dag uses function docs as doc_md for DAG object if doc_md is not explicitly set.""" | ||
|
||
@dag_decorator(default_args=self.DEFAULT_ARGS) | ||
@dag_decorator(default_args=self.DEFAULT_ARGS, doc_md=dag_doc_md) | ||
def noop_pipeline(): | ||
""" | ||
Regular DAG documentation | ||
""" | ||
|
||
@task_decorator | ||
def return_num(num): | ||
return num | ||
|
||
return_num(4) | ||
"""Regular DAG documentation""" | ||
|
||
dag = noop_pipeline() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "test" | ||
assert dag.doc_md.strip(), "Regular DAG documentation" | ||
assert dag.dag_id == "noop_pipeline" | ||
assert dag.doc_md == expected_doc_md | ||
|
||
def test_documentation_template_rendered(self): | ||
"""Test that @dag uses function docs as doc_md for DAG object""" | ||
|
@@ -2629,16 +2620,10 @@ def noop_pipeline(): | |
{% endif %} | ||
""" | ||
|
||
@task_decorator | ||
def return_num(num): | ||
return num | ||
|
||
return_num(4) | ||
|
||
dag = noop_pipeline() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "test" | ||
assert dag.doc_md.strip(), "Regular DAG documentation" | ||
assert dag.dag_id == "noop_pipeline" | ||
assert "Regular DAG documentation" in dag.doc_md | ||
|
||
def test_resolve_documentation_template_file_rendered(self): | ||
"""Test that @dag uses function docs as doc_md for DAG object""" | ||
|
@@ -2652,16 +2637,19 @@ def test_resolve_documentation_template_file_rendered(self): | |
""" | ||
) | ||
f.flush() | ||
template_dir = os.path.dirname(f.name) | ||
template_file = os.path.basename(f.name) | ||
|
||
with DAG("test-dag", start_date=DEFAULT_DATE, doc_md=template_file) as dag: | ||
task = EmptyOperator(task_id="op1") | ||
|
||
task | ||
@dag_decorator( | ||
"test-dag", start_date=DEFAULT_DATE, template_searchpath=template_dir, doc_md=template_file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test was broken even after correcting the assertion. Just needed a proper |
||
) | ||
def markdown_docs(): | ||
... | ||
|
||
assert isinstance(dag, DAG) | ||
assert dag.dag_id, "test" | ||
assert dag.doc_md.strip(), "External Markdown DAG documentation" | ||
dag = markdown_docs() | ||
assert isinstance(dag, DAG) | ||
assert dag.dag_id == "test-dag" | ||
assert dag.doc_md.strip() == "External Markdown DAG documentation" | ||
|
||
def test_fails_if_arg_not_set(self): | ||
"""Test that @dag decorated function fails if positional argument is not set""" | ||
|
@@ -2731,7 +2719,7 @@ def return_num(num): | |
|
||
self.operator.run(start_date=self.DEFAULT_DATE, end_date=self.DEFAULT_DATE) | ||
ti = dr.get_task_instances()[0] | ||
assert ti.xcom_pull(), new_value | ||
assert ti.xcom_pull() == new_value | ||
|
||
@pytest.mark.parametrize("value", [VALUE, 0]) | ||
def test_set_params_for_dag(self, value): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking against all of the doc attrs for tasks rather than just
doc_md
. Thinking users most likely wouldn't set more than one of these at a time.