jedcunningham commented on code in PR #38166:
URL: https://github.com/apache/airflow/pull/38166#discussion_r1529105471
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
+ elif keyname and (k == "val" or k == "value"):
x = secrets_masker.redact(v, keyname)
- result.append((k, x))
+ result.update({k: x})
Review Comment:
```suggestion
result[k] = x
```
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
Review Comment:
```suggestion
result[k] = v
```
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
+ elif keyname and (k == "val" or k == "value"):
x = secrets_masker.redact(v, keyname)
- result.append((k, x))
+ result.update({k: x})
keyname = None
else:
- result.append((k, v))
+ result.update({k: v})
return result
def _mask_connection_fields(extra_fields):
"""Mask connection fields."""
- result = []
- for k, v in extra_fields:
- if k == "extra":
+ result = {}
+ for k, v in extra_fields.items():
+ if k == "extra" and v:
try:
extra = json.loads(v)
- extra = [(k, secrets_masker.redact(v, k)) for k, v in
extra.items()]
- result.append((k, json.dumps(dict(extra))))
+ extra = {k: secrets_masker.redact(v, k) for k, v in
extra.items()}
+ result.update({k: dict(extra)})
except json.JSONDecodeError:
- result.append((k, "Encountered non-JSON in `extra` field"))
+ result.update({k: "Encountered non-JSON in `extra` field"})
Review Comment:
```suggestion
result[k] = "Encountered non-JSON in `extra` field"
```
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
+ elif keyname and (k == "val" or k == "value"):
x = secrets_masker.redact(v, keyname)
- result.append((k, x))
+ result.update({k: x})
keyname = None
else:
- result.append((k, v))
+ result.update({k: v})
return result
def _mask_connection_fields(extra_fields):
"""Mask connection fields."""
- result = []
- for k, v in extra_fields:
- if k == "extra":
+ result = {}
+ for k, v in extra_fields.items():
+ if k == "extra" and v:
try:
extra = json.loads(v)
- extra = [(k, secrets_masker.redact(v, k)) for k, v in
extra.items()]
- result.append((k, json.dumps(dict(extra))))
+ extra = {k: secrets_masker.redact(v, k) for k, v in
extra.items()}
+ result.update({k: dict(extra)})
Review Comment:
```suggestion
result[k] = dict(extra)
```
##########
airflow/www/decorators.py:
##########
@@ -94,35 +94,54 @@ def wrapper(*args, **kwargs):
user = get_auth_manager().get_user_name()
user_display = get_auth_manager().get_user_display_name()
- fields_skip_logging = {"csrf_token", "_csrf_token",
"is_paused"}
- extra_fields = [
- (k, secrets_masker.redact(v, k))
+ isAPIRequest = request.blueprint == "/api/v1"
+ hasJsonBody = request.headers.get("content-type") ==
"application/json" and request.json
+
+ fields_skip_logging = {
+ "csrf_token",
+ "_csrf_token",
+ "is_paused",
+ "dag_id",
+ "task_id",
+ "dag_run_id",
+ "run_id",
+ "execution_date",
+ }
+ extra_fields = {
+ k: secrets_masker.redact(v, k)
for k, v in
itertools.chain(request.values.items(multi=True), request.view_args.items())
if k not in fields_skip_logging
- ]
+ }
if event and event.startswith("variable."):
- extra_fields = _mask_variable_fields(extra_fields)
- if event and event.startswith("connection."):
- extra_fields = _mask_connection_fields(extra_fields)
+ extra_fields = _mask_variable_fields(
+ request.json if isAPIRequest and hasJsonBody else
extra_fields
+ )
+ elif event and event.startswith("connection."):
+ extra_fields = _mask_connection_fields(
+ request.json if isAPIRequest and hasJsonBody else
extra_fields
+ )
+ elif hasJsonBody:
+ extra_fields.update({"body": request.json})
Review Comment:
```suggestion
extra_fields["body"] = request.json
```
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
+ elif keyname and (k == "val" or k == "value"):
x = secrets_masker.redact(v, keyname)
- result.append((k, x))
+ result.update({k: x})
keyname = None
else:
- result.append((k, v))
+ result.update({k: v})
return result
def _mask_connection_fields(extra_fields):
"""Mask connection fields."""
- result = []
- for k, v in extra_fields:
- if k == "extra":
+ result = {}
+ for k, v in extra_fields.items():
+ if k == "extra" and v:
try:
extra = json.loads(v)
- extra = [(k, secrets_masker.redact(v, k)) for k, v in
extra.items()]
- result.append((k, json.dumps(dict(extra))))
+ extra = {k: secrets_masker.redact(v, k) for k, v in
extra.items()}
+ result.update({k: dict(extra)})
except json.JSONDecodeError:
- result.append((k, "Encountered non-JSON in `extra` field"))
+ result.update({k: "Encountered non-JSON in `extra` field"})
else:
- result.append((k, secrets_masker.redact(v, k)))
+ result.update({k: secrets_masker.redact(v, k)})
Review Comment:
```suggestion
result[k] = secrets_masker.redact(v, k)
```
##########
airflow/www/decorators.py:
##########
@@ -94,35 +94,54 @@ def wrapper(*args, **kwargs):
user = get_auth_manager().get_user_name()
user_display = get_auth_manager().get_user_display_name()
- fields_skip_logging = {"csrf_token", "_csrf_token",
"is_paused"}
- extra_fields = [
- (k, secrets_masker.redact(v, k))
+ isAPIRequest = request.blueprint == "/api/v1"
+ hasJsonBody = request.headers.get("content-type") ==
"application/json" and request.json
+
+ fields_skip_logging = {
+ "csrf_token",
+ "_csrf_token",
+ "is_paused",
+ "dag_id",
+ "task_id",
+ "dag_run_id",
+ "run_id",
+ "execution_date",
+ }
+ extra_fields = {
+ k: secrets_masker.redact(v, k)
for k, v in
itertools.chain(request.values.items(multi=True), request.view_args.items())
if k not in fields_skip_logging
- ]
+ }
if event and event.startswith("variable."):
- extra_fields = _mask_variable_fields(extra_fields)
- if event and event.startswith("connection."):
- extra_fields = _mask_connection_fields(extra_fields)
+ extra_fields = _mask_variable_fields(
+ request.json if isAPIRequest and hasJsonBody else
extra_fields
+ )
+ elif event and event.startswith("connection."):
+ extra_fields = _mask_connection_fields(
+ request.json if isAPIRequest and hasJsonBody else
extra_fields
+ )
+ elif hasJsonBody:
+ extra_fields.update({"body": request.json})
params = {**request.values, **request.view_args}
+ if params and "is_paused" in params:
+ extra_fields.update({"is_paused": params["is_paused"] ==
"false"})
Review Comment:
```suggestion
extra_fields["is_paused"] = params["is_paused"] ==
"false"
```
##########
airflow/www/decorators.py:
##########
@@ -44,36 +44,36 @@ def _mask_variable_fields(extra_fields):
Mask the 'val_content' field if 'key_content' is in the mask list.
The variable requests values and args comes in this form:
- [('key', 'key_content'),('val', 'val_content'), ('description',
'description_content')]
+ [{'key': 'key_content'},{'val': 'val_content'}, {'description':
'description_content'}]
"""
- result = []
+ result = {}
keyname = None
- for k, v in extra_fields:
+ for k, v in extra_fields.items():
if k == "key":
keyname = v
- result.append((k, v))
- elif keyname and k == "val":
+ result.update({k: v})
+ elif keyname and (k == "val" or k == "value"):
x = secrets_masker.redact(v, keyname)
- result.append((k, x))
+ result.update({k: x})
keyname = None
else:
- result.append((k, v))
+ result.update({k: v})
Review Comment:
```suggestion
result[k] = v
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
