potiuk commented on code in PR #25509:
URL: https://github.com/apache/airflow/pull/25509#discussion_r937738270
##########
airflow/models/dag.py:
##########
@@ -522,17 +522,35 @@ def __init__(
self.has_on_success_callback = self.on_success_callback is not None
self.has_on_failure_callback = self.on_failure_callback is not None
- self.doc_md = doc_md
-
self._access_control =
DAG._upgrade_outdated_dag_access_control(access_control)
self.is_paused_upon_creation = is_paused_upon_creation
self.jinja_environment_kwargs = jinja_environment_kwargs
self.render_template_as_native_obj = render_template_as_native_obj
+
+ self.doc_md = self.get_doc_md(doc_md)
+
self.tags = tags or []
self._task_group = TaskGroup.create_root(self)
self.validate_schedule_and_params()
+ def get_doc_md(self, doc_md: str) -> str:
+ if doc_md is None:
+ return doc_md
+
+ env = self.get_template_env(force_sandboxed=False)
+
+ if not doc_md.endswith('.md'):
+ template = jinja2.Template(doc_md)
+ else:
+ # template = env.loader.get_source(env, doc_md)[0]
+ template = env.get_template(doc_md)
+
+ return template.render()
Review Comment:
Double render.
##########
airflow/models/dag.py:
##########
@@ -522,17 +522,35 @@ def __init__(
self.has_on_success_callback = self.on_success_callback is not None
self.has_on_failure_callback = self.on_failure_callback is not None
- self.doc_md = doc_md
-
self._access_control =
DAG._upgrade_outdated_dag_access_control(access_control)
self.is_paused_upon_creation = is_paused_upon_creation
self.jinja_environment_kwargs = jinja_environment_kwargs
self.render_template_as_native_obj = render_template_as_native_obj
+
+ self.doc_md = self.get_doc_md(doc_md)
+
self.tags = tags or []
self._task_group = TaskGroup.create_root(self)
self.validate_schedule_and_params()
+ def get_doc_md(self, doc_md: str) -> str:
+ if doc_md is None:
+ return doc_md
+
+ env = self.get_template_env(force_sandboxed=False)
+
+ if not doc_md.endswith('.md'):
+ template = jinja2.Template(doc_md)
+ else:
+ # template = env.loader.get_source(env, doc_md)[0]
+ template = env.get_template(doc_md)
+
+ return template.render()
+
+ return template.render()
+ # return pathlib.Path(doc_md).read_text() if pathlib.Path(str(doc_md
or '')).is_file() else doc_md
Review Comment:
You can remove the comment :)
##########
docs/apache-airflow/concepts/dags.rst:
##########
@@ -564,7 +564,8 @@ doc_md markdown
doc_rst reStructuredText
========== ================
-Please note that for DAGs, ``doc_md`` is the only attribute interpreted.
+Please note that for DAGs, ``doc_md`` is the only attribute interpreted. For
DAGs it can contain a string or the reference to a template file. Template
references are recognized by str ending in ‘.md’.
Review Comment:
Also the file must exist.
##########
airflow/models/dag.py:
##########
@@ -522,17 +522,35 @@ def __init__(
self.has_on_success_callback = self.on_success_callback is not None
self.has_on_failure_callback = self.on_failure_callback is not None
- self.doc_md = doc_md
-
self._access_control =
DAG._upgrade_outdated_dag_access_control(access_control)
self.is_paused_upon_creation = is_paused_upon_creation
self.jinja_environment_kwargs = jinja_environment_kwargs
self.render_template_as_native_obj = render_template_as_native_obj
+
+ self.doc_md = self.get_doc_md(doc_md)
+
self.tags = tags or []
self._task_group = TaskGroup.create_root(self)
self.validate_schedule_and_params()
+ def get_doc_md(self, doc_md: str) -> str:
+ if doc_md is None:
+ return doc_md
+
+ env = self.get_template_env(force_sandboxed=False)
+
+ if not doc_md.endswith('.md'):
Review Comment:
```suggestion
if not doc_md.endswith('.md'):
```
I think we also nee to handle the case (and add tests). where the file is
missing (and treat it is document). I can easily imagine a case when somoene
puts a description:
"And then you might also see more in this fancy document.md")
Which would trigger a not-nice exception.
##########
airflow/models/dag.py:
##########
@@ -522,17 +522,35 @@ def __init__(
self.has_on_success_callback = self.on_success_callback is not None
self.has_on_failure_callback = self.on_failure_callback is not None
- self.doc_md = doc_md
-
self._access_control =
DAG._upgrade_outdated_dag_access_control(access_control)
self.is_paused_upon_creation = is_paused_upon_creation
self.jinja_environment_kwargs = jinja_environment_kwargs
self.render_template_as_native_obj = render_template_as_native_obj
+
+ self.doc_md = self.get_doc_md(doc_md)
+
self.tags = tags or []
self._task_group = TaskGroup.create_root(self)
self.validate_schedule_and_params()
+ def get_doc_md(self, doc_md: str) -> str:
+ if doc_md is None:
+ return doc_md
+
+ env = self.get_template_env(force_sandboxed=False)
Review Comment:
```suggestion
env = self.get_template_env(force_sandboxed=True)
```
I think we should be careful here and use sandboxed environment (unless
there is a reason we should not).
--
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]
