[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r366219349
##
File path: airflow/contrib/kubernetes/pod_launcher.py
##
@@ -122,6 +122,8 @@ def _monitor_pod(self, pod, get_logs):
if get_logs:
logs = self.read_pod_logs(pod)
for line in logs:
+if isinstance(line, bytes):
+line = line.decode('utf-8').strip()
Review comment:
Is this a related change?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger URL: https://github.com/apache/airflow/pull/6998#discussion_r366219446 ## File path: airflow/hooks/http_hook.py ## @@ -22,6 +22,7 @@ import requests import tenacity +from airflow import conf Review comment: Is this a related change? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r365956885
##
File path: airflow/utils/log/logging_mixin.py
##
@@ -109,7 +110,10 @@ def _propagate_log(self, message):
"""
Propagate message removing escape codes.
"""
-self.logger.log(self.level, remove_escape_codes(message))
+if conf.getboolean('core', 'colored_ui_logs'):
+self.logger.log(self.level, message)
+else:
+self.logger.log(self.level, remove_escape_codes(message))
Review comment:
Got it. I am just wondering if colored logs in ui shouldn't be default.
@potiuk @mik-laj WDYT?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r365592307
##
File path: airflow/utils/log/logging_mixin.py
##
@@ -109,7 +110,10 @@ def _propagate_log(self, message):
"""
Propagate message removing escape codes.
"""
-self.logger.log(self.level, remove_escape_codes(message))
+if conf.getboolean('core', 'colored_ui_logs'):
+self.logger.log(self.level, message)
+else:
+self.logger.log(self.level, remove_escape_codes(message))
Review comment:
If ASCI codes are handled properly in WebUI I don't think we need this case?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r365592237
##
File path: airflow/hooks/http_hook.py
##
@@ -142,8 +144,13 @@ def check_response(self, response):
try:
response.raise_for_status()
except requests.exceptions.HTTPError:
-self.log.error("HTTP error: %s", response.reason)
-self.log.error(response.text)
+# mark the exception if colored log is supported
+if conf.getboolean('core', 'colored_ui_logs'):
+self.log.error(Fore.WHITE + Back.RED + "HTTP error: %s",
response.reason + Style.RESET_ALL)
+self.log.error(Fore.WHITE + Back.RED + response.text +
Style.RESET_ALL)
Review comment:
If I am not mistaken the log will be colored two times? Once by color log
formatter, the second time by adding codes here?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r363417731
##
File path: airflow/www/templates/airflow/ti_log.html
##
@@ -123,8 +123,32 @@ {{ title }}
if(auto_tailing && checkAutoTailingCondition()) {
var should_scroll = true
}
+
+// ANSI color support
+var ansi_color_regex = /.\[([0-9][0-9])m(.*?).\[0m/gs;
+var res.message = res.message.replace(ansi_color_regex,
function(match, color, content) {
+ var colors = {
+30: 'black', 40: 'black',
+31: 'red',41: 'red',
+32: 'green', 42: 'green',
+33: 'yellow', 43: 'yellow',
+34: 'blue', 44: 'blue',
+35: 'magenta',45: 'magenta',
+36: 'cyan', 46: 'cyan',
+37: 'white', 47: 'white'
+ }
+
+ if (30 <= parseInt(color) && parseInt(color) <= 37) {
Review comment:
@mik-laj can you advise?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] - ANSI color support for WebUI logger
nuclearpinguin commented on a change in pull request #6998: [AIRFLOW-6404] -
ANSI color support for WebUI logger
URL: https://github.com/apache/airflow/pull/6998#discussion_r362482454
##
File path: airflow/www/templates/airflow/ti_log.html
##
@@ -123,8 +123,32 @@ {{ title }}
if(auto_tailing && checkAutoTailingCondition()) {
var should_scroll = true
}
+
+// ANSI color support
+var ansi_color_regex = /.\[([0-9][0-9])m(.*?).\[0m/gs;
+var res.message = res.message.replace(ansi_color_regex,
function(match, color, content) {
+ var colors = {
+30: 'black', 40: 'black',
+31: 'red',41: 'red',
+32: 'green', 42: 'green',
+33: 'yellow', 43: 'yellow',
+34: 'blue', 44: 'blue',
+35: 'magenta',45: 'magenta',
+36: 'cyan', 46: 'cyan',
+37: 'white', 47: 'white'
+ }
+
+ if (30 <= parseInt(color) && parseInt(color) <= 37) {
Review comment:
+1 for using existing solution
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:
us...@infra.apache.org
With regards,
Apache Git Services
