o-nikolas commented on code in PR #53459:
URL: https://github.com/apache/airflow/pull/53459#discussion_r2216934519


##########
task-sdk/src/airflow/sdk/bases/operator.py:
##########
@@ -1200,6 +1203,27 @@ def __hash__(self):
                 hash_components.append(repr(val))
         return hash(tuple(hash_components))
 
+    def validate_executor_config(self, executor_config: dict | None) -> None:
+        """
+        Validate the executor_config to ensure it only contains 'pod_override' 
or 'pod_template_file' keys.
+
+        :param executor_config: The executor_config dictionary to validate.
+        :raises AirflowException: If executor_config contains keys other than 
'pod_override' or 'pod_template_file'.
+        """
+        if not executor_config or not isinstance(executor_config, dict):
+            return
+
+        valid_keys = {"pod_override", "pod_template_file"}
+        invalid_keys = set(executor_config.keys()) - valid_keys
+        if invalid_keys:
+            error_msg = (
+                f"Invalid executor_config keys for task '{self.task_id}'"
+                f"{' in DAG ' + self.dag.dag_id if self.has_dag() else ''}: 
{sorted(invalid_keys)}. "
+                f"Only 'pod_override' and 'pod_template_file' are allowed."
+            )
+            self.log.error(error_msg)
+            raise AirflowException(error_msg)

Review Comment:
   Whoa, whoa, whoa, thanks for tagging me @amoghrajesh we do **not** want to 
do this.
   
   We have many executors other than KubernetesExecutor, so at the base class 
level we absolutely should not assume/assert anything about the executor_config.
   
   For example the ECS executor treats the executor_config as kwargs for the 
ECS run_task api call, to allow users to set overrides for the parameters that 
API accepts.
   
   If we want to do any validation it's going to have to be a lot more 
complicated to try detect the executor being used, which feels like a lot of 
work and I expect it to be a little brittle. You could also do validation at 
task queued time, which would at least bring it a bit earlier.



-- 
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]

Reply via email to