Add task documentation to details tab in grid view. #39899
Conversation
tirkarthi
requested review from
ryanahamilton,
ashb,
bbovenzi,
pierrejeambrun and
ephraimbuddy
as code owners
boring-cyborg
bot
added
area:API
|
Let's rebase and see if that fixes the CI errors? |
There was a problem hiding this comment.
Thanks for the contribution! Three small things you might change if you need to fix the tests anyway.
Before merge I'd like to ask if we need to support the other doc formats as well. Currently in the "legacy" views as described in https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/dags.html#dag-task-documentation it is also supported to add plain text, JSON, YAML and RST - I am not sure whether somebody really uses these but the current version only supports MD. I am okay with this but the documentation might need to be adjusted. At least the legacy pages show also the JSON, YAML, plain text and RST (as plain text though).
Can I kindly request to adjust the docs?
airflow/www/static/js/dag/details/taskInstance/TaskDocumentation.tsx
Outdated
Show resolved
Hide resolved
|
Thanks @jscheffl for pointing that out. I tried making something like a multi level accordion but it looks cluttered to me. Also there is json renderer used for dag run conf but rst might need a renderer. I feel this needs a separate discussion of it's own to build the UX and probably taken as a separate PR with doc_md being the more common one useful in this PR.
|
|
@bbovenzi I have rebased with latest main branch but the tests still seem to fail due to fab provider related issues which are not related to the PR. |
There was a problem hiding this comment.
Then... okay yeah also don't want to generate a (new) cluttered view. Was mainly looking at documentation which pointed to more options than markdown.
For me it is okay to adjust the documentation in a follow-up clean PR where the legacy pages are removed and thus the documentation display either need to be extended or documentation adjusted that only doc_md will be displayed on UI.
Let's go with just doc_md. The UI is allowed to have opinions sometimes. |
|
Fab tests keep failing though other PRs seem to pass after multiple rebases to this PR. Any idea? |
|
Let's rebase again and see if the test failures persists? |
|
I also noticed report where one another PR also has failing provider tests. |
Hope that #40059 fixed it. |
|
I have rebased and it still fails. |
|
One more fix is needed (I wrongly used return instead of exit for the check): #40089 |
tirkarthi
force-pushed
the
grid-task-doc
branch
2 times, most recently
from
0fe3719
to
2f3df3c
Compare
|
GREEN - ready to get merged (finally)? |
|
Thank you very much all for constant updates to the PR. Happy to see this finally merged. |
* Add task documentation to details tab in grid view. * Fix tests. * Fix tests. * Fix PR comments.
closes: #39676
closes: #12750
related: #39676
This PR enhances the task detail API to return doc_md in API. It follows a similar approach to task dependency check component to hit the API and display the documentation as an accordion like notes so that it can be expanded/collapsed as needed.
Documentation accordion expanded
Documentation accordion collapsed