Add XCom tab to Grid #35719

hduong-mwam · 2023-11-18T12:24:05Z

Overview

Added XCom tab on Grid view which uses the XCom endpoints from REST API.

Tasks with no XCom

Dynamic task mapping

Implementation notes

React query hooks to fetch XCom keys and values (two separate endpoints)
Loading and errors are displayed similar to other component implementations
Entries rendered in tabular format within Charkra UI Box to allow scrolling on long XCom

^ 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.rst or {issue_number}.significant.rst, in newsfragments.

boring-cyborg · 2023-11-18T12:24:09Z

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
Be sure to read the Airflow Coding style.
Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack

airflow/www/static/js/dag/details/index.tsx

bbovenzi · 2023-11-20T14:11:28Z

airflow/www/static/js/dag/details/taskInstance/Xcom/XcomEntry.tsx

+    tryNumber: tryNumber || 1,
+  });
+
+  let content = <Text fontFamily="monospace">{xcom?.value}</Text>;


Could you test this with an extremely large json text blob?

I tested with 49344 bytes of lorem ipsum generated from https://www.lipsum.com/ and it seems to render out fine. Do you think this is a reasonable test?

That's good but do you mind putting in a check to see if the value is valid json and then we can format it accordingly?

Sorry I am afraid I am not following. In my test, I just have a task which returns this dict {"long_string": "Lorem..."}. This results in two XCom entries:

The key long_string: Lorem...

And the full return_value: {"long_string": "Lorem..."}

Do you mean check that the entry value is a valid JSON and format that? I personally haven't have a use-case that I return an entry that is a JSON but instead if the task returns a dict then I can access the attributes as keys (if that makes sense)

Keyed portion

Full return_value portion

(Apologies for the handwriting 😄) Here is an example of shorter XCom but with many keys to illustrate my point

I agree with the table showing key and value as separate columns. I meant to say that we could try to check if the value follows any forma and then render it accordingly. It's an additional nice-to-have. We can do it in another PR.

Ah, I get you. Would be a nice addition for sure

airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx

bbovenzi · 2023-11-20T14:17:04Z

Thanks so much for doing it. I just have a few comments, but overall looks good!

jscheffl

I really love the UI changes gradually migrating from legacy to new.

When I tested locally I saw still the old link pointing to the old XCom page in the second row of Task details in Grid. As below.

Can you remove this and redirect the route from /xcom to the grid sub-page as well?

As release 2.8.0 is just about to be cut, if finished it might still be elegible into the next version, would be cool.

bbovenzi · 2023-11-20T23:11:53Z

I really love the UI changes gradually migrating from legacy to new.

When I tested locally I saw still the old link pointing to the old XCom page in the second row of Task details in Grid. As below.

Can you remove this and redirect the route from /xcom to the grid sub-page as well?

As release 2.8.0 is just about to be cut, if finished it might still be elegible into the next version, would be cool.

Yes, we should remove that link.

hduong-mwam · 2023-11-26T22:15:57Z

I really love the UI changes gradually migrating from legacy to new.

When I tested locally I saw still the old link pointing to the old XCom page in the second row of Task details in Grid. As below.

Can you remove this and redirect the route from /xcom to the grid sub-page as well?

As release 2.8.0 is just about to be cut, if finished it might still be elegible into the next version, would be cool.

I have removed the link from the Details tab in Grid view.

However, I have not added redirect for /xcom. I still see the /task, /rendered-template, /log endpoints being available. Any suggestions on whether I should remove the xcom tab here entirely or we leave it as is similar to log. There are no direct links from grid view to this page as far as I know and we only get to this page via the "More Detail" button

jscheffl · 2023-11-26T23:23:34Z

I really love the UI changes gradually migrating from legacy to new.
When I tested locally I saw still the old link pointing to the old XCom page in the second row of Task details in Grid. As below.
Can you remove this and redirect the route from /xcom to the grid sub-page as well?
As release 2.8.0 is just about to be cut, if finished it might still be elegible into the next version, would be cool.

I have removed the link from the Details tab in Grid view.

However, I have not added redirect for /xcom. I still see the /task, /rendered-template, /log endpoints being available. Any suggestions on whether I should remove the xcom tab here entirely or we leave it as is similar to log. There are no direct links from grid view to this page as far as I know and we only get to this page via the "More Detail" button

I'd propose to remove the other link. Would it be possible to redirect the /xcom route like /code to the new view?
(But my proposal is just "my" opinion, how about the others?)

bbovenzi · 2023-11-27T15:37:53Z

We can keep the old xcom page for now. I think we can have separate PRs to update the grid task instance details to match the legacy page and then add a rendered template tab. Once we have those we can just delete that entire set of pages.

airflow/www/static/js/dag/details/index.tsx

…tance

bbovenzi

Just tested locally. It looks great!

boring-cyborg · 2023-12-04T15:59:40Z

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

hduong-mwam · 2023-12-04T16:11:20Z

Thank you for approving and merging. It is my first PR so I am very grateful for the comments and the warm reception. Was a blast working on the PR, can't wait for it to be in the next release!

* Add XCom tab to Grid * Combine showLogs and showXcom logic evaluation to isIndividualTaskInstance * Remove link to /xcom page from UI grid view * Use consistent naming to distinguish XcomCollection and XcomEntry * Refactor boolean vars (cherry picked from commit 77c0103)

hduong-mwam requested review from ryanahamilton, ashb, bbovenzi and pierrejeambrun as code owners November 18, 2023 12:24

boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Nov 18, 2023

hduong-mwam mentioned this pull request Nov 18, 2023

Display selected task outputs (xcom) in task UI #26905

Closed

2 tasks

bbovenzi reviewed Nov 20, 2023

View reviewed changes

airflow/www/static/js/dag/details/index.tsx Outdated Show resolved Hide resolved

bbovenzi reviewed Nov 20, 2023

View reviewed changes

airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx Outdated Show resolved Hide resolved

bbovenzi added this to the Airflow 2.8.0 milestone Nov 20, 2023

jscheffl reviewed Nov 20, 2023

View reviewed changes

hduong-mwam force-pushed the feature-xcom-grid-branch branch from 258c47f to 4ed45d0 Compare November 26, 2023 21:12

bbovenzi reviewed Nov 27, 2023

View reviewed changes

airflow/www/static/js/dag/details/index.tsx Outdated Show resolved Hide resolved

bbovenzi reviewed Nov 27, 2023

View reviewed changes

airflow/www/static/js/dag/details/index.tsx Outdated Show resolved Hide resolved

hduong-mwam added 5 commits December 2, 2023 10:24

Add XCom tab to Grid

05de510

Combine showLogs and showXcom logic evaluation to isIndividualTaskIns…

170847b

…tance

Remove link to /xcom page from UI grid view

824049d

Use consistent naming to distinguish XcomCollection and XcomEntry

b2b6fab

Refactor boolean vars

fec1bae

hduong-mwam force-pushed the feature-xcom-grid-branch branch from 74c3deb to fec1bae Compare December 2, 2023 11:27

bbovenzi approved these changes Dec 4, 2023

View reviewed changes

bbovenzi merged commit 77c0103 into apache:main Dec 4, 2023
47 checks passed

ephraimbuddy added the type:new-feature Changelog: New Features label Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XCom tab to Grid #35719

Add XCom tab to Grid #35719

hduong-mwam commented Nov 18, 2023 •

edited by eladkal

boring-cyborg bot commented Nov 18, 2023

bbovenzi Nov 20, 2023

hduong-mwam Nov 26, 2023

bbovenzi Nov 27, 2023 •

edited

hduong-mwam Dec 2, 2023

bbovenzi Dec 4, 2023

hduong-mwam Dec 4, 2023

bbovenzi commented Nov 20, 2023

jscheffl left a comment

bbovenzi commented Nov 20, 2023

hduong-mwam commented Nov 26, 2023

jscheffl commented Nov 26, 2023

bbovenzi commented Nov 27, 2023

bbovenzi left a comment

boring-cyborg bot commented Dec 4, 2023

hduong-mwam commented Dec 4, 2023

Add XCom tab to Grid #35719

Add XCom tab to Grid #35719

Conversation

hduong-mwam commented Nov 18, 2023 • edited by eladkal

Overview

Implementation notes

boring-cyborg bot commented Nov 18, 2023

bbovenzi Nov 20, 2023

Choose a reason for hiding this comment

hduong-mwam Nov 26, 2023

Choose a reason for hiding this comment

bbovenzi Nov 27, 2023 • edited

Choose a reason for hiding this comment

hduong-mwam Dec 2, 2023

Choose a reason for hiding this comment

bbovenzi Dec 4, 2023

Choose a reason for hiding this comment

hduong-mwam Dec 4, 2023

Choose a reason for hiding this comment

bbovenzi commented Nov 20, 2023

jscheffl left a comment

Choose a reason for hiding this comment

bbovenzi commented Nov 20, 2023

hduong-mwam commented Nov 26, 2023

jscheffl commented Nov 26, 2023

bbovenzi commented Nov 27, 2023

bbovenzi left a comment

Choose a reason for hiding this comment

boring-cyborg bot commented Dec 4, 2023

hduong-mwam commented Dec 4, 2023

hduong-mwam commented Nov 18, 2023 •

edited by eladkal

bbovenzi Nov 27, 2023 •

edited