josh-fell commented on code in PR #29502:
URL: https://github.com/apache/airflow/pull/29502#discussion_r1106546805


##########
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:
   >I am now of an opinion that constructors in our operators should just 
assign fields. Full stop.
   
   Yes, same. I guess I figured since this was a hook, we're not moving the 
connection call out, and it's currently not going to get called in an operator, 
why not push up the validation? But I see it was misguided suggestion.
   
   > ...get rid of explicit constructor and turn all our operators in 
'dataclasses'
   
   I would be over the moon if this was the implementation or `attrs`; the 
latter has been life changing. It's a shame you can't build an operator now, 
with any real added value in simplication, with `attrs` because of the 
metaclass logic going on. Alas, I would love this.



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