Add asset events to dashboard #44961
Conversation
tirkarthi
requested review from
bbovenzi,
pierrejeambrun,
ryanahamilton,
jscheffl and
ephraimbuddy
as code owners
tirkarthi
force-pushed
the
dashboard-events
branch
from
c2076ee to
f69cf68
Compare
| @@ -102,6 +102,7 @@ class AssetEventResponse(BaseModel): | |||
|
|
|||
| id: int | |||
| asset_id: int | |||
| uri: str | None = Field(alias="uri", default=None) | |||
There was a problem hiding this comment.
if we're to add uri here, we'll probably need to add name (and maybe group?)
There was a problem hiding this comment.
I’m not sure we should add uri here. If we need it in the UI, it’d be better to include the entire asset object instead.
There was a problem hiding this comment.
This is as per the initial UI design for dashboard. I am okay with changing this as per consensus.
There was a problem hiding this comment.
The designs were quite preliminary so I am happy with adding everything suggested
| data-testid="asset-sort-duration" | ||
| defaultValue={["-timestamp"]} | ||
| onValueChange={(option) => setAssetSortBy(option.value[0] as string)} | ||
| variant="outline" // Flushed seems to be right option to remove border but is not valid as per typescript |
There was a problem hiding this comment.
What does this comment mean?
There was a problem hiding this comment.
In the UI design the select component has no border in asset events widget. "flushed" is a variant that achieves this effect on select but the type for select only has outline | subtle as valid values though flushed works. This can be kept outline too and would just deviate from the design.
There was a problem hiding this comment.
I don't think "flushed" is a valid or at least not a publically-supported variant. So we can remove the comment. And we can add a borderWidth={0} param
|
Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted. |
| <Time datetime={event.timestamp} /> | ||
| </Text> | ||
| <HStack> | ||
| <FiDatabase /> <Text> {event.uri ?? ""} </Text> |
There was a problem hiding this comment.
Likely better to use the name here, maybe we some rich UI that reveals more asset information on user interaction.
There was a problem hiding this comment.
Once we pass name and group we can put more useful information here. And could have a tooltip to show more.
Add asset events to dashboard to display the top 6 events sorted by default newest first (latest) asset events. The source links to the task instance the created the event. The selected time from filter is applied to the API to get only events in last 12 hours, past week etc. The triggered dagruns links to the dagruns triggered due to the asset event.
Notes to reviewer and self
from_rest_apiit's usually from API and source is displayed as API. Similar can be done for asset events from trigger. Opened Add from_trigger field to AssetEvent extra for events from Trigger #44944 for discussion.asset.urifor the card. Do we need to displayasset.nameorasset.uri?Related #42700