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 2022/08/15 23:02:09 UTC
[GitHub] [airflow] pierrejeambrun opened a new pull request, #25731: Use cfg default_wrap value for grid logs
pierrejeambrun opened a new pull request, #25731:
URL: https://github.com/apache/airflow/pull/25731
Related: https://github.com/apache/airflow/issues/24721
Thanks @bbovenzi for the ping. Here is a quick PR that also uses the `default_wrap` param value for the grid logs.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946210598
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Does `tojson` convert to boolean?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946223008
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
I could replace this with something more explicit: (but still not ideal, I find the string quotes confusing)
```
const defaultWrap = {{ 'true' if default_wrap else 'false' }};
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946214337
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946223008
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
I could replace this with something more explicit:
```
const defaultWrap = {{ 'true' if default_wrap else 'false' }};
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun closed pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun closed pull request #25731: Use cfg default_wrap value for grid logs
URL: https://github.com/apache/airflow/pull/25731
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946214337
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946233524
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Actually, we should put this in a `<meta>` tag vs directly accessing here. and then in the js file we can call `const defaultWrap = getMetaValue('default_wrap') === 'True';`
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Actually, we should put this in a `<meta>` tag vs directly accessing here. and then in the js file we can call ```const defaultWrap = getMetaValue('default_wrap') === 'True';```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi merged pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #25731:
URL: https://github.com/apache/airflow/pull/25731
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946212868
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Yes, I get an error without it, he is trying to access a variable named `True`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r946212868
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Yes, I get an error without it, he is trying to access a variable named `True` and `False` respectively.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #25731: Use cfg default_wrap value for grid logs
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #25731:
URL: https://github.com/apache/airflow/pull/25731#discussion_r947037386
##########
airflow/www/templates/airflow/grid.html:
##########
@@ -42,6 +42,7 @@
const stateColors = {{ state_color_mapping|tojson }};
const autoRefreshInterval = {{ auto_refresh_interval }};
const defaultDagRunDisplayNumber = {{ default_dag_run_display_number }};
+ const defaultWrap = {{ default_wrap|tojson }};
Review Comment:
Done :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org