You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "josh-fell (via GitHub)" <gi...@apache.org> on 2023/02/07 05:10:12 UTC

[GitHub] [airflow] josh-fell opened a new pull request, #29399: Persist DAG and task doc values in TaskFlow API if explicitly set

josh-fell opened a new pull request, #29399:
URL: https://github.com/apache/airflow/pull/29399

   If users set `doc_md` arg on `@dag`- or `@task`-decorated TaskFlow functions and those functions have a docstring, the `doc_md` value is not respected. Instead this PR will enforce only using the TaskFlow function docstring as the documentation if `doc_md` (or any `doc*` attrs for tasks) is not set.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell commented on a diff in pull request #29399: Persist DAG and task doc values in TaskFlow API if explicitly set

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29399:
URL: https://github.com/apache/airflow/pull/29399#discussion_r1098184890


##########
tests/models/test_dag.py:
##########
@@ -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

Review Comment:
   This test was broken even after correcting the assertion. Just needed a proper `template_searchpath` arg.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell commented on a diff in pull request #29399: Persist DAG and task doc values in TaskFlow API if explicitly set

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29399:
URL: https://github.com/apache/airflow/pull/29399#discussion_r1098184082


##########
airflow/decorators/base.py:
##########
@@ -315,7 +315,9 @@ def __call__(self, *args: FParams.args, **kwargs: FParams.kwargs) -> XComArg:
             multiple_outputs=self.multiple_outputs,
             **self.kwargs,
         )
-        if self.function.__doc__:
+        op_doc_attrs = [op.doc, op.doc_json, op.doc_md, op.doc_rst, op.doc_yaml]
+        # Set the task's doc_md to the function's docstring if it exists and no other doc* args are set.
+        if self.function.__doc__ and not any(op_doc_attrs):

Review Comment:
   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.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] josh-fell commented on a diff in pull request #29399: Persist DAG and task doc values in TaskFlow API if explicitly set

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #29399:
URL: https://github.com/apache/airflow/pull/29399#discussion_r1098184436


##########
tests/models/test_dag.py:
##########
@@ -2565,58 +2565,49 @@ def noop_pipeline():
 
         dag = noop_pipeline()
         assert isinstance(dag, DAG)
-        assert dag.dag_id, "noop_pipeline"

Review Comment:
   Typical example of improper assertion that is being fixed in this PR.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk merged pull request #29399: Persist DAG and task doc values in TaskFlow API if explicitly set

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29399:
URL: https://github.com/apache/airflow/pull/29399


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org