uranusjr commented on code in PR #38015:
URL: https://github.com/apache/airflow/pull/38015#discussion_r1521946748
##########
airflow/decorators/base.py:
##########
@@ -208,11 +208,35 @@ def __init__(
# since values for those will be provided when the task is run. Since
# we're not actually running the function, None is good enough here.
signature = inspect.signature(python_callable)
+
+ # Don't allow context argument defaults other than None to avoid
ambiguities.
+ if any(
+ param.name in KNOWN_CONTEXT_KEYS and param.default not in (None,
inspect.Parameter.empty)
+ for param in signature.parameters.values()
+ ):
+ raise ValueError("Context key parameters can't have a default
other than None")
+
parameters = [
param.replace(default=None) if param.name in KNOWN_CONTEXT_KEYS
else param
for param in signature.parameters.values()
]
- signature = signature.replace(parameters=parameters)
+ try:
+ signature = signature.replace(parameters=parameters)
+ except ValueError as err:
+ message = textwrap.dedent(
+ f"""
+ The function signature broke while assigning defaults to
context key parameters.
+
+ The decorator is replacing the signature
+ > {python_callable.__name__}({', '.join(str(param) for param
in signature.parameters.values())})
+
+ with
+ > {python_callable.__name__}({', '.join(str(param) for param
in parameters)})
+
+ which isn't valid: {err}
+ """
+ )
+ raise ValueError(message)
Review Comment:
```suggestion
raise ValueError(message) from err
```
Cleaener traceback
##########
airflow/decorators/base.py:
##########
@@ -208,11 +208,35 @@ def __init__(
# since values for those will be provided when the task is run. Since
# we're not actually running the function, None is good enough here.
signature = inspect.signature(python_callable)
+
+ # Don't allow context argument defaults other than None to avoid
ambiguities.
+ if any(
+ param.name in KNOWN_CONTEXT_KEYS and param.default not in (None,
inspect.Parameter.empty)
+ for param in signature.parameters.values()
+ ):
+ raise ValueError("Context key parameters can't have a default
other than None")
+
parameters = [
param.replace(default=None) if param.name in KNOWN_CONTEXT_KEYS
else param
for param in signature.parameters.values()
]
- signature = signature.replace(parameters=parameters)
+ try:
+ signature = signature.replace(parameters=parameters)
+ except ValueError as err:
+ message = textwrap.dedent(
+ f"""
+ The function signature broke while assigning defaults to
context key parameters.
+
+ The decorator is replacing the signature
+ > {python_callable.__name__}({', '.join(str(param) for param
in signature.parameters.values())})
+
+ with
+ > {python_callable.__name__}({', '.join(str(param) for param
in parameters)})
+
+ which isn't valid: {err}
+ """
+ )
+ raise ValueError(message)
Review Comment:
```suggestion
raise ValueError(message) from err
```
Cleaner traceback
--
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]
