Validate Hive Beeline parameters

[potiuk]

The parameters for Hive when beeline is used should be validated in order to avoid unnecessary jdbc calls when they are invalid.

This PR adds raising an exception early in cases the parameters are not correct.

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[pankajastro]

LGTM.

[josh-fell]

Would it make sense to push this validation to the constructor? The Connection object is retrieved there as well as determining the use_beeline attr. Could fail faster before even executing the task. WDYT?

[pankajastro]

hmm, didn't notice maybe makes sense here since we have conn in __init__.
Not related to changes in this PR but should we think of moving self.get_connection(hive_cli_conn_id) call outside __init__ otherwise it will make a db call to get conn whenever dag parsing will happen?

[josh-fell]

The db call would only happen if an operator using this hook constructed the hook in its __init__. There is almost an instance of that in the HiveOperator, but it's to setup lazy loading the hook and make it an instance attr.

But yes, ideally the constructors of hooks, operators, sensor, etc. don't make any external calls

[potiuk]

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.

[Taragolis]

I agree that we need to avoid anything which might cause call to Airflow DB or any other resource.
There is situation when hook initialised in operator not lazily and even expected as one of argument:
SSHHook and SSHOperator

Also I agree that operator should only allow fields declared however even if I'am a fan of dataclasses there is couple of issues exists:

1. __post_init__, there will be temptation to use it.
2. A bit dumb inheritance, you should decorate child class with @dataclass otherwise it turned into the regular class with part of dataclass 🙄
3. kwargs-only dataclasses introduced only in Python 3.10+

[josh-fell]

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.

[potiuk]

Alas, I would love this.

I share the sentiment :)