[GitHub] [airflow] potiuk commented on a diff in pull request #29502: Validate Hive Beeline parameters

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on code in PR #29502:
URL: https://github.com/apache/airflow/pull/29502#discussion_r1106514891


##########
airflow/providers/apache/hive/hooks/hive.py:
##########
@@ -141,6 +141,7 @@ def _prepare_cli_cmd(self) -> list[Any]:
 
         if self.use_beeline:
             hive_bin = "beeline"
+            self._validate_beeline_parameters(conn)

Review Comment:
   Additionally, If you initialize connection in __init__ you cannot make connection_id templated - even if you extend the operator.
   
   While I used to think the same @josh-fell that early validation helps to surface such problems earlier, I think It has evolved and I am now of an opinion that constructors in our operators should **just** assign fields. Full stop.
   
   And in the past (cannot find it easily) @uranusjr proposed that we get rid of explicit constructor and turn all our operators in 'dataclasses' https://peps.python.org/pep-0557/ - where you have only fields declared and all the __init__ boilerplate is generated.
   
   And I tend to agree that would be a good idea. 



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