airflow
airflow copied to clipboard
Grid logs for mapped instances
https://github.com/apache/airflow/pull/25568 added support for retrieving mapped instance task logs.
This PR adds support for displaying these logs in the Grid details side panel.
- Mapped Instances rows in the table are not selectable anymore. (Actions are done on all of them)
- Mapped Instances rows are clickable and bring a new details page for mapped instances. (We can see the Map Index on top of other detailed information).
- For the new details, logs tab is available and show the logs for this particular map index.
- Breadcrumbs get extended with the map index and the
taskInstance
breadcrumbs becomes clickable in this particular case. - Extracting tasks actions into its own component.
- Add a button to go back to the dynamic task summary (thanks @bbovenzi)
- Add navigation link the same way we have for normal taks (xcom, details, templates, ...) and remove those links from MappedInstance Table (thanks @bbovenzi)
- Move the MappedInstance Table to its own tab (again thanks @bbovenzi)
- Added a few unit tests.
Tested on:
- Mapped Tasks (obviously) :heavy_check_mark:
- Groups with mapped task :heavy_check_mark:
- Logs display,
See More
andDownload
links, and logs filters :heavy_check_mark: - When there is multiple pages in the
MappedInstances
Table :heavy_check_mark: - Breadcrumbs navigation :heavy_check_mark:
- Task actions for all mapped task are working :heavy_check_mark:
- Task actions from the details of a mapped task instance does not work with the
SequentialExecutor
but the calls seems fine. (I didn't modify that part)
Notes: This adds logic to the TaskInstance
which is already quite complicated. I believe it went from 'complexe' to 'too complexe', but I do not see a trivial way to refactor it. I do not want to add a lot of changes on top of that PR, I'll most certainly open a follow up PR to this end. Let me know what you think about that.
Also fixes: https://github.com/apache/airflow/issues/25616
I updated the PR and the description.
-
Nav link is done, and removed them from the table, nice suggestion. :) I'm not sure about direct log access, I feel like it is already covered by the preferred tab. If the user preferred tab is 'logs" he will directly land on logs by clicking on the table row otherwise he will land on details.
-
Added the button to go back to the dynamic task summary, good idea!
-
Separate the mapped task table on a new tab only available for mapped task summary. Unfortunately I do not have the number of mapped instance info available atm. (this could be an improvement).
Logs
& Mapped Tasks
tab are mutually exclusive. In terms of 'preferredTab' they act as the same. Meaning you can go from 'mapped' -> 'logs' and reverse without changing tabs.
Since we store the map_index in the url, we need to make sure we load an individual mapped task instance details. Try refreshing the page when you have a single mapped task selected and the details panel will be blank.
Also, a task instance might not have logs (ie: it is scheduled but hasn't run yet). We should handle that case instead of just showing a blank page to a user.
Have you tested this with auto-refresh on? I don't think the mapped instance details are updating.
Since we store the map_index in the url, we need to make sure we load an individual mapped task instance details. Try refreshing the page when you have a single mapped task selected and the details panel will be blank.
I broke that in my last commit :facepalm:... sorry for that.
Maybe this is the source of some of the confusion. (I checked auto refresh for mapped task and tasks without logs, it's working now).
I don't think we should be using
useEffect
here. The Details component should probably get the mapped instance info from the API:/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}
Good idea that would indeed simplify things.
~~Converted to draft while I implement the suggested change to the API.~~
edit: This endpoint already exists get_mapped_task_instance
With your suggestions I was able to come up with a much simpler implementation, that I feel will be more robust as well.
Thanks
Thank you Brent :)