Remove host as an instance attr in DatabricksHook

[josh-fell]

As part of #20526, parsing logic of host was moved out of the __init__() method in DatabricksHook. With this change, host no longer needs to be an instance attribute of the hook.

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

[josh-fell]

CC @dstandish

[potiuk]

We can include it in the next provider's released (planned to do it tomorrow.)

[github-actions]

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

[dstandish]

I like it... BUT... must we consider this a "breaking" change?

[josh-fell]

I like it... BUT... must we consider this a "breaking" change?

For some historical context on this and moving the get_connection() call in and out of __init__() if it helps assist in a decision:

• v2.0.1
  • get_connection() call exists in __init__()
  • host is not an instance attr
• v2.0.2
  • get_connection() removed __init__()
  • host is not an instance attr
• v2.1.0
  • get_connection() was put back into __init__() via Refactor DatabricksHook #19835 which then added host as an instance attr
  • get_connection() was then removed again __init__() via Remove db call from DatabricksHook.__init__() #20180 which kept host as an instance attr
  • Ultimately, end state was get_connection() removed from __init__() but host remained an instance attr
• Since the last provider release:
  • get_connection() moved back into __init__() to address Mypy reenablement errors in Fix MyPy Errors for Databricks provider. #20265
  • get_connection() called as part of a cached prop in Update connection object to cached_property in DatabricksHook #20526
  • host remained untouched until this PR

Quite the adventure.

[potiuk]

I like it... BUT... must we consider this a "breaking" change?

Why ?

[dstandish]

I like it... BUT... must we consider this a "breaking" change?

Why ?

Because host will no longer be set after __init__

in last release it seems instance attr host was set in __init__

this among the most marginal of changes we should consider "breaking" but if we want to be strict it probably is right to consider it so. in this case the solution should probably be to make it a cached property. but @potiuk if you think we can just chop the attr that would be best from code perspective. based on @josh-fell's analyis it seems it was only an instance attr for one release (since v2.1.0)

[potiuk]

I think "behavrioural" changes like that are not "breaking". Whether self.host is an instance of variable does not change the "use" of it - at most it optimizes some behaviours - in this case the "self.host" was created in both - DagParser and Task, when it was removed, it was only created in Task (during execute).

As we discussed here: #19572 (comment) - Databricks is not really a "generic" operator that you might want to extend or dertive from to use internal features. Unlke KPO for example which is generic and it's easier to imagine that people would like to change their behaviour by extending it. So I think we can consider it's internals are really "implementation details".

Unfortunately we do not have clear-cut what is breaking and what not :(. Theoretically even adding a default parameter might be breaking because somone could overload that method in a derived class, or you could imagine someone checks signature of the method.

This is really almost always an exercise of "how likely this change is breaking" rather than "clear-cut" 0/1 breaking/non-breaking. And except some obvious cases, almost always it will have to be judged individually.

[dstandish]

thanks @potiuk