Skip to content

Fix truncated execution_date in UI#32465

Closed
renzepost wants to merge 3 commits intoapache:mainfrom
renzepost:execution_dates_truncated
Closed

Fix truncated execution_date in UI#32465
renzepost wants to merge 3 commits intoapache:mainfrom
renzepost:execution_dates_truncated

Conversation

@renzepost
Copy link
Contributor

@renzepost renzepost commented Jul 9, 2023

Add data-attribute to time elements in HTML. Configure datetime formatting based on this attribute, all instances of execution_date and dag_run.execution_date will be formatted as isoformat with milliseconds and timezone. Microseconds precision is unfortunately not possible, because the datetimes are formatted in Javascript.

closes: #28482
Examples:

Task instance details:
image

DAG runs:
image

Task Instances:
image

Task instance logs (nothing changed here):
image

DAG Audit log:
image


^ 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 boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Jul 9, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if this is alright or if there's a better solution than this. I focused on solving the issue as described in #28482, and probably overlooked a lot of things.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should do this to all execution_date fields that are displayed in the UI. WDYT?

https://github.com/search?q=repo%3Aapache%2Fairflow+execution_date+language%3AJavaScript&type=code&l=JavaScript

Also, since there are multiple places we are displaying it. We should have common logic to control data-datetime-convert=False attribute in the view layer for execution_date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good idea. I need to rework this a little too. With this solution, the execution date won't be updated when the user changes the display timezone. However, JS date types don't have microsecond precision. So currently the best that can be done is millisecond precision for execution date when displayed in the UI, while at the same time being able to switch display timezone. It won't satisfy the bug report completely though.
Another thing I overlooked is that if data-datetime-convert is true (or unset), the title attribute of the time element will be filled with the datetime in UTC, if the display timezone is set to non-UTC. This solution would break that behavior for execution date too.
Let me know your thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did some rework on this. See updated description of PR. Let me know if I missed any instances of execution_date in the UI!

@eladkal eladkal added this to the Airflow 2.6.4 milestone Jul 12, 2023
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Jul 12, 2023
@renzepost renzepost force-pushed the execution_dates_truncated branch from 141a949 to a67ac94 Compare July 13, 2023 18:59
@renzepost renzepost changed the title Fix truncated execution_date in task instance details Fix truncated execution_date in UI Jul 13, 2023
@pierrejeambrun
Copy link
Member

pierrejeambrun commented Jul 23, 2023

I find it weird that we have the execution_date formatted differently than all other dates (start_date, end_date, etc...) in the UI.

In the original issue, the date displayed at the top of the taskinstance page is just the 'time', meant by the at, an approximate human readable time. The run_id is explicitely available a few lines bellow.

I don't think this piece of information is supposed to be used to retrieve the run_id, mainly because the run_id could be completely unrelated to the execution_date. (manually triggered run from the UI or Rest API)

@renzepost
Copy link
Contributor Author

I'm ok with closing this, but then I guess the original bug ticket should be closed too.

@pierrejeambrun
Copy link
Member

Lets see what others think

@eladkal eladkal modified the milestones: Airflow 2.6.4, Airflow 2.7.0 Aug 1, 2023
@bbovenzi
Copy link
Contributor

bbovenzi commented Aug 2, 2023

Yeah, the "Task Id at time" is supposed to just be something more human readable. I'd rather not have too many different date formats without it being clear to a user.Maybe we need to have the run_id appear towards the top of the list of task instance details?

Perhaps we should choose whether execution date or run id should be the unique key to show in a table or to search by.

@bbovenzi bbovenzi removed this from the Airflow 2.7.0 milestone Aug 2, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Sep 18, 2023
@github-actions github-actions bot closed this Sep 24, 2023
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 stale Stale PRs per the .github/workflows/stale.yml policy file type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Execution dates are truncated on the UI

5 participants