Fix markdown rendering on dag docs #50142
Conversation
aritra24
requested review from
bbovenzi,
pierrejeambrun,
ryanahamilton,
jscheffl and
shubhamraj-git
as code owners
|
I would prefer the backend to trim/dedent on docstring during dag parsing than frontend. There could be code blocks with indent in docstring and trimming leading space might cause issues. |
|
Yeah, I agree. I wasn't sure what would be the smartest way of trimming. I can move it to the backend and perhaps do a strip min(whitespace at the front of lines)? What do you think @tirkarthi |
There was a problem hiding this comment.
I think doc_md attribute manually shouldn't contain any leading space (that's how the doc and internet resource specify that) for instance:
https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/dags.html#dag-task-documentation.
But this is not possible when de doc is coming from the docstring. I think we should remove leading spaces there when parsing the docstring. (this way what's stored in the db is correct and the API and UI will work without further modification )
|
@pierrejeambrun, moved it to the server side. Let me know what you think now? |
|
Changing the milestone to 3.0.2 -- about to cut 3.0.1 |
aritra24
requested review from
ephraimbuddy,
rawwar,
jason810496 and
bugraoz93
as code owners
pierrejeambrun
added
the
backport-to-v3-0-test
(cherry picked from commit d1ef9b9) Co-authored-by: Aritra Basu <[email protected]>
Backport successfully created: v3-0-test
|
(cherry picked from commit d1ef9b9) Co-authored-by: Aritra Basu <[email protected]>
(cherry picked from commit d1ef9b9) Co-authored-by: Aritra Basu <[email protected]>
(cherry picked from commit d1ef9b9) Co-authored-by: Aritra Basu <[email protected]>
| @@ -164,6 +165,14 @@ def get_timezone(cls, tz: Timezone | FixedTimezone) -> str | None: | |||
| return None | |||
| return str(tz) | |||
|
|
|||
| @field_validator("doc_md", mode="before") | |||
| @classmethod | |||
| def get_doc_md(cls, doc_md: str | None) -> str | None: | |||
There was a problem hiding this comment.
Sorry, coming "late" to the party.... and just one tought... would it not be better to "clean" the dedent at time of DAG parsing, not at point of retrieval? That would prevent potential problems of the docs are consumed in other scenarios as well.
There was a problem hiding this comment.
I'm not opposed to that, it reduces the number of calls to dedent as well. I think the reason behind this was to not effect the serialized db entry.
before and after
Not sure if there is a case where stripping the whitespace in the front changes the expected experience.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.