Skip to content

Conversation

@pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented Jun 6, 2022

closes: #23503

First draft to discuss changes and precise UI requirements for this.

Some of the changes:

  • Add tabs (Details & Logs)
  • Clicking on the attempt index would load the relevant logs (and not download the logs)
  • We keep a download button for downloading the logs of the currently selected attempt.
  • We add a button 'See more' or 'Details' that would redirect to the log page. (Same as the Nav button)
  • Split metadata details into two columns for smaller vertical space
  • Checkboxes for full log content and text wrapping

For the request URL, I can pass it into the metadata to be able to do something like:

const url = getMetaValue('task_instance_log_api')

But then I have to manually replace _DAG_RUN_ID, _TASK_ID_, _TASK_TRY_NUMBER. What's the best way to handle that ?

image


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 newsfragement file, named {pr_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Jun 6, 2022
@pierrejeambrun pierrejeambrun force-pushed the 23503-task-logs-to-grid-details-panel branch from 94fe6e0 to 701ccc9 Compare June 7, 2022 21:13
@pierrejeambrun
Copy link
Member Author

I just updated the PR, as suggested:

  • Added tabs to improve visibility. (More tabs are to come)
  • Persisted the user preference (tab selection) into the local storage. Only when the preferedTab is not available, we will default to the Detail tabs, but won't persist this fallback. This way we keep the user preference for later when the tab is available again.

Also added:

  • A checkbox to request the full log, using the query param already implemented on the endpoint (full_content)

Best,
image
image

@pierrejeambrun pierrejeambrun force-pushed the 23503-task-logs-to-grid-details-panel branch 3 times, most recently from 97bc171 to 8b14dbd Compare June 8, 2022 19:03
@bbovenzi
Copy link
Contributor

bbovenzi commented Jun 8, 2022

In a separate PR, before or after, we should make the grid only as big as it needs to be and the details panel can grow to take up the rest of the page (inverse of today). The logs textarea gets cramped as it is now.

@pierrejeambrun pierrejeambrun force-pushed the 23503-task-logs-to-grid-details-panel branch from 551e858 to 41302cc Compare June 8, 2022 21:39
@NilsJPWerner
Copy link
Contributor

In a separate PR, before or after, we should make the grid only as big as it needs to be and the details panel can grow to take up the rest of the page (inverse of today). The logs textarea gets cramped as it is now.

It might be cool to make the panel resizeable.

@pierrejeambrun pierrejeambrun force-pushed the 23503-task-logs-to-grid-details-panel branch from cbf78ad to d31b1ed Compare June 12, 2022 13:20
Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work!

@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Jun 12, 2022
@pierrejeambrun pierrejeambrun force-pushed the 23503-task-logs-to-grid-details-panel branch from 4dad6f7 to 30935ba Compare June 12, 2022 18:21
@pierrejeambrun
Copy link
Member Author

@bbovenzi,

Thank you for the reviews and the improvement suggestions.

The branch is rebased :)

@bbovenzi bbovenzi merged commit 770ee07 into apache:main Jun 12, 2022
@pierrejeambrun pierrejeambrun deleted the 23503-task-logs-to-grid-details-panel branch June 12, 2022 19:51
@NilsJPWerner
Copy link
Contributor

Will this included in 2.3.4 or a 2.4.0?

@bbovenzi bbovenzi added this to the Airflow 2.4.0 milestone Jul 14, 2022
@bbovenzi
Copy link
Contributor

bbovenzi commented Jul 14, 2022

@NilsJPWerner 2.4.0, its certainly not a bug fix that we can sneak into a patch release. but 2.4.0 will come out far quicker than 2.3.0 did.

@NilsJPWerner
Copy link
Contributor

Fantastic! I'm pumped for this feature! Do you think the graphs refresh will make it in as well?

@bbovenzi
Copy link
Contributor

That's the plan!

@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues okay to merge It's ok to merge this PR as it does not require more tests type:new-feature Changelog: New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Task Logs to Grid details panel

4 participants