Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add display_name field to graphs endpoint #8287

Closed
wants to merge 1 commit into from
Closed

add display_name field to graphs endpoint #8287

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/prefect/orion/models/flow_runs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ async def read_flow_runs(
class DependencyResult(PrefectBaseModel):
id: UUID
name: str
display_name: str
upstream_dependencies: List[TaskRunResult]
state: State
expected_start_time: Optional[datetime.datetime]
Expand Down Expand Up @@ -351,6 +352,7 @@ async def read_task_run_dependencies(
"state": task_run.state,
"expected_start_time": task_run.expected_start_time,
"name": task_run.name,
"display_name": task_run.task_key.rpartition(".")[-1],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we partition on a period?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, if anything, I think the display name should be a portion of the task run name not the task key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The task_key is delimited by a ., in instances where there is no period, like when a user has defined a key manually, rpartition will still result in an array where the last value is the whole word.

Copy link
Contributor

@zanieb zanieb Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I see. The task_key defaults to the qualified import name of the task, a right-partition on a period does make sense here. However, the task key doesn't really make sense as the display name for a task run since task runs won't be distinguishable? A right partition of the task_key is also the same as the default value for the task name so if you are just trying to show task names you should probably use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. If there are more than one instance of a task run in a flow, it will show up twice with the same name, but the intent is for the UI to surface the unique name on click. The unique string at the end is not very human readable on it's own and clutters charts, making it more harmful than helpful when simply fallowing the series of events.

You can see that demonstrated here:
https://user-images.githubusercontent.com/6776415/215178374-c839420b-5bbd-461b-9501-2a602846e9e9.mov

The individual task run name will be available when exploring the task details.

"start_time": task_run.start_time,
"end_time": task_run.end_time,
"total_run_time": task_run.total_run_time,
Expand Down