You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/05/27 20:52:47 UTC
[GitHub] [airflow] bbovenzi opened a new pull request #16129: Format more dates with timezone
bbovenzi opened a new pull request #16129:
URL: https://github.com/apache/airflow/pull/16129
A bunch of dates in the UI were not being formatted with the user's selected timezone from the dropdown in top navigation. I went through and updated as many as I could find.
One exception, for some reason, the task instance start date had an explicit comment to not format it, https://github.com/apache/airflow/blame/10ed42a837e11d8e954c1f885e289a4248edd2ca/airflow/www/static/js/task_instances.js#L82, @ashb should that remain the case?
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton merged pull request #16129: Format more dates with timezone
Posted by GitBox <gi...@apache.org>.
ryanahamilton merged pull request #16129:
URL: https://github.com/apache/airflow/pull/16129
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on pull request #16129: Format more dates with timezone
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #16129:
URL: https://github.com/apache/airflow/pull/16129#issuecomment-850516467
@ryanahamilton good call. It's all updated now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on a change in pull request #16129: Format more dates with timezone
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #16129:
URL: https://github.com/apache/airflow/pull/16129#discussion_r641083047
##########
File path: airflow/www/templates/airflow/dag_details.html
##########
@@ -50,11 +50,11 @@ <h3>{{ title }}</h3>
</tr>
<tr>
<th>Start Date</th>
- <td>{{ dag.start_date }}</td>
+ <td class="format-date">{{ dag.start_date }}</td>
Review comment:
```suggestion
<td class="js-format-date">{{ dag.start_date }}</td>
```
##########
File path: airflow/www/static/js/task.js
##########
@@ -17,11 +17,18 @@
* under the License.
*/
-/* global document */
-import { formatDateTime } from './datetime_utils';
+/* global document, moment */
-// reformat execution date to be more human-readable
+// reformat task details to be more human-readable
document.addEventListener('DOMContentLoaded', () => {
- const date = document.getElementById('ti_execution_date');
- date.innerHTML = formatDateTime(date.innerHTML);
+ document.querySelectorAll('.ti-attr').forEach((attr) => {
+ const value = attr.innerHTML;
+ if (value.length === 32 && moment(value, 'YYYY-MM-DD').isValid()) {
+ // 32 is the length of our timestamps
+ attr.className = 'format-date';
Review comment:
```suggestion
attr.className = 'js-format-date';
```
##########
File path: airflow/www/templates/airflow/dag_details.html
##########
@@ -50,11 +50,11 @@ <h3>{{ title }}</h3>
</tr>
<tr>
<th>Start Date</th>
- <td>{{ dag.start_date }}</td>
+ <td class="format-date">{{ dag.start_date }}</td>
</tr>
<tr>
<th>End Date</th>
- <td>{{ dag.end_date }}</td>
+ <td class="format-date">{{ dag.end_date }}</td>
Review comment:
```suggestion
<td class="js-format-date">{{ dag.end_date }}</td>
```
##########
File path: airflow/www/templates/airflow/task_instance.html
##########
@@ -26,7 +26,7 @@
<br>
<h4>
<span class="text-muted">Task Instance:</span> <span>{{ task_id }}</span>
- <span class="text-muted">at</span> <span id="ti_execution_date">{{ execution_date }}</span>
+ <span class="text-muted">at</span> <span class="format-date">{{ execution_date }}</span>
Review comment:
```suggestion
<span class="text-muted">at</span> <span class="js-format-date">{{ execution_date }}</span>
```
##########
File path: airflow/www/static/js/ti_log.js
##########
@@ -113,7 +115,9 @@ function autoTailingLog(tryNumber, metadata = null, autoTailing = false) {
// The message may contain HTML, so either have to escape it or write it as text.
const escapedMessage = escapeHtml(item[1]);
- const linkifiedMessage = escapedMessage.replace(urlRegex, (url) => `<a href="${url}" target="_blank">${url}</a>`);
+ const linkifiedMessage = escapedMessage
+ .replace(urlRegex, (url) => `<a href="${url}" target="_blank">${url}</a>`)
+ .replaceAll(dateRegex, (date) => `<span class="format-date">${formatDateTime(`${date}+00:00`)}</span>`);
Review comment:
```suggestion
.replaceAll(dateRegex, (date) => `<span class="js-format-date">${formatDateTime(`${date}+00:00`)}</span>`);
```
##########
File path: airflow/www/static/js/datetime_utils.js
##########
@@ -88,6 +88,12 @@ export function updateAllDateTimes() {
$('.datetime input').each((_, el) => {
el.value = moment(el.value).format();
});
+
+ $('.format-date').each((_, el) => {
Review comment:
```suggestion
$('.js-format-date').each((_, el) => {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16129: Format more dates with timezone
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16129:
URL: https://github.com/apache/airflow/pull/16129#issuecomment-850601614
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 master or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on a change in pull request #16129: Format more dates with timezone
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #16129:
URL: https://github.com/apache/airflow/pull/16129#discussion_r641736594
##########
File path: airflow/www/static/js/task.js
##########
@@ -17,11 +17,18 @@
* under the License.
*/
-/* global document */
-import { formatDateTime } from './datetime_utils';
+/* global document, moment */
-// reformat execution date to be more human-readable
+// reformat task details to be more human-readable
document.addEventListener('DOMContentLoaded', () => {
- const date = document.getElementById('ti_execution_date');
- date.innerHTML = formatDateTime(date.innerHTML);
+ document.querySelectorAll('.ti-attr').forEach((attr) => {
Review comment:
Sorry I didn't catch this last time, but should we change this one as well?
```suggestion
document.querySelectorAll('.js-ti-attr').forEach((attr) => {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org